From c5b2f503806de206b65919b28fec01e1da32336a Mon Sep 17 00:00:00 2001 From: John Nicholson Date: Fri, 20 Mar 2026 03:51:40 -0400 Subject: [PATCH 1/6] show progress arc in map view while waiting for first cloudslam map While a job is active but no map has been received yet, PointCloudMap returns a PCD point arc instead of the blank default. The arc grows from a small sliver to a full circle over 5 minutes, giving the user visual feedback that the session is running. Co-Authored-By: Claude Sonnet 4.6 --- cloudslam/cloudslam.go | 51 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/cloudslam/cloudslam.go b/cloudslam/cloudslam.go index 85f6ea2..650ad4c 100644 --- a/cloudslam/cloudslam.go +++ b/cloudslam/cloudslam.go @@ -7,16 +7,18 @@ import ( "embed" "errors" "fmt" + "math" + "os" "strconv" "strings" "sync/atomic" "time" - "os" - + "github.com/golang/geo/r3" pbCloudSLAM "go.viam.com/api/app/cloudslam/v1" "go.viam.com/rdk/grpc" "go.viam.com/rdk/logging" + "go.viam.com/rdk/pointcloud" "go.viam.com/rdk/resource" "go.viam.com/rdk/services/slam" "go.viam.com/rdk/spatialmath" @@ -72,6 +74,7 @@ type cloudslamWrapper struct { resource.AlwaysRebuild activeJob atomic.Pointer[string] + jobStartTime atomic.Pointer[time.Time] lastPose atomic.Pointer[spatialmath.Pose] lastPointCloudURL atomic.Pointer[string] defaultpcd []byte @@ -296,8 +299,16 @@ func (svc *cloudslamWrapper) Position(ctx context.Context) (spatialmath.Pose, er func (svc *cloudslamWrapper) PointCloudMap(ctx context.Context, returnEditedMap bool) (func() ([]byte, error), error) { currMap := *svc.lastPointCloudURL.Load() - // return the placeholder map when no maps are present if currMap == "" { + startTime := svc.jobStartTime.Load() + if startTime != nil { + progressPCD, err := generateProgressRingPCD(time.Since(*startTime)) + if err != nil { + svc.logger.Warnf("failed to generate progress PCD: %v", err) + return toChunkedFunc(svc.defaultpcd), nil + } + return toChunkedFunc(progressPCD), nil + } return toChunkedFunc(svc.defaultpcd), nil } pcdBytes, err := svc.app.GetDataFromHTTP(ctx, currMap) @@ -340,6 +351,8 @@ func (svc *cloudslamWrapper) DoCommand(ctx context.Context, req map[string]inter return nil, err } svc.activeJob.Store(&jobID) + startTime := time.Now() + svc.jobStartTime.Store(&startTime) svc.lastPose.Store(&initPose) svc.lastPointCloudURL.Store(&initPCDURL) @@ -354,6 +367,7 @@ func (svc *cloudslamWrapper) DoCommand(ctx context.Context, req map[string]inter if err != nil { return nil, err } + svc.jobStartTime.Store(nil) resp[stopJobKey] = "Job completed, find your map at " + packageURL } if packageName, ok := req[localPackageKey]; ok { @@ -480,6 +494,37 @@ func (svc *cloudslamWrapper) ParseSensorsForPackage() ([]interface{}, error) { return sensorMetadata, nil } +// generateProgressRingPCD generates a point cloud of a progress arc indicating elapsed time. +// The arc grows clockwise from 0 to a full circle over progressRingDuration, giving the user +// visual feedback while waiting for the first cloudslam map to appear. +func generateProgressRingPCD(elapsed time.Duration) ([]byte, error) { + const ( + numPoints = 360 + radius = 1000.0 // mm + progressRingDuration = 5 * time.Minute + ) + + // Always show at least a small arc so the user sees something immediately. + fraction := math.Max(0.02, math.Min(elapsed.Seconds()/progressRingDuration.Seconds(), 1.0)) + filledPoints := int(fraction * numPoints) + + pc := pointcloud.NewBasicEmpty() + for i := 0; i < filledPoints; i++ { + angle := float64(i) / numPoints * 2 * math.Pi + x := radius * math.Cos(angle) + y := radius * math.Sin(angle) + if err := pc.Set(r3.Vector{X: x, Y: y, Z: 0}, pointcloud.NewBasicData()); err != nil { + return nil, err + } + } + + var buf bytes.Buffer + if err := pointcloud.ToPCD(pc, &buf, pointcloud.PCDAscii); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + // toChunkedFunc takes binary data and wraps it in a helper function that converts it into chunks for streaming APIs. func toChunkedFunc(b []byte) func() ([]byte, error) { chunk := make([]byte, chunkSizeBytes) From 950a83cb6b0adddc554199c89b8dc3bf14e8774b Mon Sep 17 00:00:00 2001 From: John Nicholson Date: Fri, 20 Mar 2026 04:54:25 -0400 Subject: [PATCH 2/6] fix url --- cloudslam/cloudslam.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudslam/cloudslam.go b/cloudslam/cloudslam.go index 650ad4c..ae90c4a 100644 --- a/cloudslam/cloudslam.go +++ b/cloudslam/cloudslam.go @@ -403,7 +403,7 @@ func (svc *cloudslamWrapper) StopJob(ctx context.Context) (string, error) { } packageName := strings.Split(resp.GetPackageId(), "/")[1] - packageURL := svc.app.baseURL + "/robots?name=" + packageName + "&version=" + resp.GetVersion() + packageURL := svc.app.baseURL + "/robots?page=slam&name=" + packageName + "&version=" + resp.GetVersion() return packageURL, nil } From cad2b04e83c5ae18fc7fdde56d567214298c4223 Mon Sep 17 00:00:00 2001 From: John Nicholson Date: Fri, 20 Mar 2026 12:54:33 -0400 Subject: [PATCH 3/6] tweak comments --- cloudslam/app.go | 1 - cloudslam/cloudslam.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cloudslam/app.go b/cloudslam/app.go index d1fa89b..cf4d55d 100644 --- a/cloudslam/app.go +++ b/cloudslam/app.go @@ -113,7 +113,6 @@ func (app *AppClient) CheckSensorsDataCapture(ctx context.Context, partID string return err } - // index sensor names for quick lookup, track which ones we've verified pending := make(map[string]struct{}, len(sensors)) for _, s := range sensors { pending[s.name] = struct{}{} diff --git a/cloudslam/cloudslam.go b/cloudslam/cloudslam.go index ae90c4a..667cd6e 100644 --- a/cloudslam/cloudslam.go +++ b/cloudslam/cloudslam.go @@ -90,7 +90,7 @@ type cloudslamWrapper struct { viamVersion string // optional cloudslam setting, describes which viam-server appimage to use(stable/latest/pr/pinned) slamVersion string // optional cloudslam setting, describes which cartographer appimage to use(stable/latest/pr/pinned) - // updating mode values. A user can only use updating mode if the partID is configured + // updating mode values. updatingMapName string // empty if slam is not in updating mode updatingMapVersion string // empty if slam is not in updating mode From c1e8517a7a5f9626ed79f104e0f36dbc11f5febe Mon Sep 17 00:00:00 2001 From: John Nicholson Date: Fri, 20 Mar 2026 13:44:53 -0400 Subject: [PATCH 4/6] claude self review --- cloudslam/app.go | 5 +---- cloudslam/cloudslam.go | 8 +++----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/cloudslam/app.go b/cloudslam/app.go index cf4d55d..5dc3f26 100644 --- a/cloudslam/app.go +++ b/cloudslam/app.go @@ -114,12 +114,9 @@ func (app *AppClient) CheckSensorsDataCapture(ctx context.Context, partID string } pending := make(map[string]struct{}, len(sensors)) - for _, s := range sensors { - pending[s.name] = struct{}{} - } - sensorTypes := make(map[string]slam.SensorType, len(sensors)) for _, s := range sensors { + pending[s.name] = struct{}{} sensorTypes[s.name] = s.sensorType } diff --git a/cloudslam/cloudslam.go b/cloudslam/cloudslam.go index 667cd6e..47f32fe 100644 --- a/cloudslam/cloudslam.go +++ b/cloudslam/cloudslam.go @@ -341,10 +341,8 @@ func (svc *cloudslamWrapper) Close(ctx context.Context) error { func (svc *cloudslamWrapper) DoCommand(ctx context.Context, req map[string]interface{}) (map[string]interface{}, error) { resp := map[string]interface{}{} if name, ok := req[startJobKey]; ok { - if svc.partID != "" { - if err := svc.app.CheckSensorsDataCapture(ctx, svc.partID, svc.sensors, svc.logger); err != nil { - return nil, err - } + if err := svc.app.CheckSensorsDataCapture(ctx, svc.partID, svc.sensors, svc.logger); err != nil { + return nil, err } jobID, isUpdating, err := svc.StartJob(svc.cancelCtx, name.(string)) if err != nil { @@ -367,7 +365,6 @@ func (svc *cloudslamWrapper) DoCommand(ctx context.Context, req map[string]inter if err != nil { return nil, err } - svc.jobStartTime.Store(nil) resp[stopJobKey] = "Job completed, find your map at " + packageURL } if packageName, ok := req[localPackageKey]; ok { @@ -402,6 +399,7 @@ func (svc *cloudslamWrapper) StopJob(ctx context.Context) (string, error) { return "", fmt.Errorf("cloudslam session failed: %s", metaResp.GetSessionMetadata().GetErrorMsg()) } + svc.jobStartTime.Store(nil) packageName := strings.Split(resp.GetPackageId(), "/")[1] packageURL := svc.app.baseURL + "/robots?page=slam&name=" + packageName + "&version=" + resp.GetVersion() return packageURL, nil From 7d5eec09dab9144a15da50b2dbebaa6438c10493 Mon Sep 17 00:00:00 2001 From: John Nicholson Date: Fri, 20 Mar 2026 13:55:16 -0400 Subject: [PATCH 5/6] clean up wrapper state - merge activeJob + jobStartTime into activeJobState struct; nil pointer replaces empty-string sentinel, and startedAt replaces the separate jobStartTime atomic so both fields are updated atomically - wrap updatingMapName/updatingMapVersion into updatingMapInfo struct; nil replaces the != "" guard and fixes a latent bug where version could be set with an empty name - clear activeJob in StopJob (stops polling) while keeping lastPointCloudURL so the final map stays visible after a session ends - store logger on AppClient at construction; drop logger param from CheckSensorsDataCapture Co-Authored-By: Claude Sonnet 4.6 --- cloudslam/app.go | 10 +++--- cloudslam/cloudslam.go | 74 ++++++++++++++++++++++++------------------ 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/cloudslam/app.go b/cloudslam/app.go index 5dc3f26..77fb6ee 100644 --- a/cloudslam/app.go +++ b/cloudslam/app.go @@ -28,7 +28,8 @@ type AppClient struct { PackageClient pbPackage.PackageServiceClient SyncClient pbDataSync.DataSyncServiceClient RobotClient pbApp.RobotServiceClient - HTTPClient *http.Client // used for downloading pcds of the current cloudslam session + HTTPClient *http.Client // used for downloading pcds of the current cloudslam session + logger logging.Logger } // CreateCloudSLAMClient creates a new grpc cloud configured to communicate with the robot service based on the cloud config given. @@ -63,6 +64,7 @@ func CreateCloudSLAMClient(ctx context.Context, apiKey, apiKeyID, baseURL string // This might be redundant with CloseIdleConnections in Close(), // and unsure if the extra cost of redoing the TLS handshake makes this change worth it HTTPClient: &http.Client{Transport: &http.Transport{DisableKeepAlives: true}}, + logger: logger, }, nil } @@ -106,7 +108,7 @@ func (app *AppClient) GetDataFromHTTP(ctx context.Context, dataURL string) ([]by // CheckSensorsDataCapture verifies that all of the provided sensors have at least one enabled // data capture method configured in the machine part's config. Returns an error listing any sensors // that are missing enabled capture. -func (app *AppClient) CheckSensorsDataCapture(ctx context.Context, partID string, sensors []*cloudslamSensorInfo, logger logging.Logger) error { +func (app *AppClient) CheckSensorsDataCapture(ctx context.Context, partID string, sensors []*cloudslamSensorInfo) error { req := pbApp.ConfigRequest{Id: partID} resp, err := app.RobotClient.Config(ctx, &req) if err != nil { @@ -124,9 +126,9 @@ func (app *AppClient) CheckSensorsDataCapture(ctx context.Context, partID string if _, ok := pending[comp.GetName()]; !ok { continue } - logger.Debugf("checking data capture for sensor %q (type %v)", comp.GetName(), sensorTypes[comp.GetName()]) + app.logger.Debugf("checking data capture for sensor %q (type %v)", comp.GetName(), sensorTypes[comp.GetName()]) for _, svcConfig := range comp.GetServiceConfigs() { - logger.Debugf(" service config type: %q, attributes: %v", svcConfig.GetType(), svcConfig.GetAttributes()) + app.logger.Debugf(" service config type: %q, attributes: %v", svcConfig.GetType(), svcConfig.GetAttributes()) } if hasEnabledDataCapture(comp, sensorTypes[comp.GetName()]) { delete(pending, comp.GetName()) diff --git a/cloudslam/cloudslam.go b/cloudslam/cloudslam.go index 47f32fe..e15e61e 100644 --- a/cloudslam/cloudslam.go +++ b/cloudslam/cloudslam.go @@ -69,12 +69,24 @@ type Config struct { BaseURL string `json:"base_url,omitempty"` // this should only be used for testing in staging } +// activeJobState holds the current job ID and when it was started. +// startedAt is zero if the job was already running when the wrapper started up. +type activeJobState struct { + id string + startedAt time.Time +} + +// updatingMapInfo holds the name and version of an existing map to continue from. +type updatingMapInfo struct { + name string + version string +} + type cloudslamWrapper struct { resource.Named resource.AlwaysRebuild - activeJob atomic.Pointer[string] - jobStartTime atomic.Pointer[time.Time] + activeJob atomic.Pointer[activeJobState] // nil when no job is running lastPose atomic.Pointer[spatialmath.Pose] lastPointCloudURL atomic.Pointer[string] defaultpcd []byte @@ -90,9 +102,7 @@ type cloudslamWrapper struct { viamVersion string // optional cloudslam setting, describes which viam-server appimage to use(stable/latest/pr/pinned) slamVersion string // optional cloudslam setting, describes which cartographer appimage to use(stable/latest/pr/pinned) - // updating mode values. - updatingMapName string // empty if slam is not in updating mode - updatingMapVersion string // empty if slam is not in updating mode + updatingMap *updatingMapInfo // nil if not in updating mode // app clients for talking to app app *AppClient @@ -229,8 +239,7 @@ func newSLAM( func (svc *cloudslamWrapper) initialize(mappingMode slam.MappingMode) error { var err error svc.lastPose.Store(&initPose) - initJob := "" - svc.activeJob.Store(&initJob) + svc.activeJob.Store(nil) svc.lastPointCloudURL.Store(&initPCDURL) // using this as a placeholder image. need to determine the right way to have the module use it @@ -244,10 +253,13 @@ func (svc *cloudslamWrapper) initialize(mappingMode slam.MappingMode) error { // the webapp does not remove the package from the config when swapping from updating mode to mapping mode, so this code // needs the extra check to ensure we only update maps when the user wants to. if svc.partID != "" && mappingMode != slam.MappingModeNewMap { - svc.updatingMapName, svc.updatingMapVersion, err = svc.app.GetSLAMMapPackageOnRobot(svc.cancelCtx, svc.partID) + name, version, err := svc.app.GetSLAMMapPackageOnRobot(svc.cancelCtx, svc.partID) if err != nil { return err } + if name != "" { + svc.updatingMap = &updatingMapInfo{name: name, version: version} + } } // check if the machine has an active job @@ -256,8 +268,9 @@ func (svc *cloudslamWrapper) initialize(mappingMode slam.MappingMode) error { if err != nil { return err } - sessionID := resp.GetSessionId() - svc.activeJob.Store(&sessionID) + if resp.GetSessionId() != "" { + svc.activeJob.Store(&activeJobState{id: resp.GetSessionId()}) + } svc.workers = goutils.NewBackgroundStoppableWorkers(svc.activeMappingSessionThread) return nil @@ -270,14 +283,14 @@ func (svc *cloudslamWrapper) activeMappingSessionThread(ctx context.Context) { return } - currJob := *svc.activeJob.Load() + job := svc.activeJob.Load() // do nothing if no active jobs - if currJob == "" { + if job == nil { continue } // get the most recent pointcloud and position if there is an active job - req := &pbCloudSLAM.GetMappingSessionPointCloudRequest{SessionId: currJob} + req := &pbCloudSLAM.GetMappingSessionPointCloudRequest{SessionId: job.id} resp, err := svc.app.CSClient.GetMappingSessionPointCloud(ctx, req) if err != nil { svc.logger.Error(err) @@ -300,9 +313,9 @@ func (svc *cloudslamWrapper) PointCloudMap(ctx context.Context, returnEditedMap currMap := *svc.lastPointCloudURL.Load() if currMap == "" { - startTime := svc.jobStartTime.Load() - if startTime != nil { - progressPCD, err := generateProgressRingPCD(time.Since(*startTime)) + job := svc.activeJob.Load() + if job != nil && !job.startedAt.IsZero() { + progressPCD, err := generateProgressRingPCD(time.Since(job.startedAt)) if err != nil { svc.logger.Warnf("failed to generate progress PCD: %v", err) return toChunkedFunc(svc.defaultpcd), nil @@ -327,7 +340,7 @@ func (svc *cloudslamWrapper) Properties(ctx context.Context) (slam.Properties, e } func (svc *cloudslamWrapper) Close(ctx context.Context) error { - if *svc.activeJob.Load() != "" { + if svc.activeJob.Load() != nil { _, err := svc.StopJob(ctx) if err != nil { svc.logger.Errorf("error while stopping job: %v", err) @@ -341,22 +354,20 @@ func (svc *cloudslamWrapper) Close(ctx context.Context) error { func (svc *cloudslamWrapper) DoCommand(ctx context.Context, req map[string]interface{}) (map[string]interface{}, error) { resp := map[string]interface{}{} if name, ok := req[startJobKey]; ok { - if err := svc.app.CheckSensorsDataCapture(ctx, svc.partID, svc.sensors, svc.logger); err != nil { + if err := svc.app.CheckSensorsDataCapture(ctx, svc.partID, svc.sensors); err != nil { return nil, err } jobID, isUpdating, err := svc.StartJob(svc.cancelCtx, name.(string)) if err != nil { return nil, err } - svc.activeJob.Store(&jobID) - startTime := time.Now() - svc.jobStartTime.Store(&startTime) + svc.activeJob.Store(&activeJobState{id: jobID, startedAt: time.Now()}) svc.lastPose.Store(&initPose) svc.lastPointCloudURL.Store(&initPCDURL) if isUpdating { resp[updatingModeKey] = fmt.Sprintf("slam map found on machine, starting cloudslam in updating mode. Map "+ - "Name = %v // Updating Version = %v", svc.updatingMapName, svc.updatingMapVersion) + "Name = %v // Updating Version = %v", svc.updatingMap.name, svc.updatingMap.version) } resp[startJobKey] = "Starting cloudslam session, the machine should appear in ~1 minute. Job ID: " + jobID } @@ -379,27 +390,26 @@ func (svc *cloudslamWrapper) DoCommand(ctx context.Context, req map[string]inter // StopJob stops the current active cloudslam job. func (svc *cloudslamWrapper) StopJob(ctx context.Context) (string, error) { - // grab the active job but do not clear it from the module. that way users can still see the final map on the machine - currJob := *svc.activeJob.Load() - if currJob == "" { + job := svc.activeJob.Load() + if job == nil { return "", errors.New("no active jobs") } - req := &pbCloudSLAM.StopMappingSessionRequest{SessionId: currJob} + req := &pbCloudSLAM.StopMappingSessionRequest{SessionId: job.id} resp, err := svc.app.CSClient.StopMappingSession(ctx, req) if err != nil { return "", err } metaResp, err := svc.app.CSClient.GetMappingSessionMetadataByID(ctx, - &pbCloudSLAM.GetMappingSessionMetadataByIDRequest{SessionId: currJob}) + &pbCloudSLAM.GetMappingSessionMetadataByIDRequest{SessionId: job.id}) if err != nil { - svc.logger.Warnf("could not retrieve session metadata for job %s: %v", currJob, err) + svc.logger.Warnf("could not retrieve session metadata for job %s: %v", job.id, err) } else if metaResp.GetSessionMetadata().GetEndStatus() == pbCloudSLAM.EndStatus_END_STATUS_FAIL { return "", fmt.Errorf("cloudslam session failed: %s", metaResp.GetSessionMetadata().GetErrorMsg()) } - svc.jobStartTime.Store(nil) + svc.activeJob.Store(nil) packageName := strings.Split(resp.GetPackageId(), "/")[1] packageURL := svc.app.baseURL + "/robots?page=slam&name=" + packageName + "&version=" + resp.GetVersion() return packageURL, nil @@ -434,9 +444,9 @@ func (svc *cloudslamWrapper) StartJob(ctx context.Context, mapName string) (stri Sensors: svc.sensorInfoToProto(), SlamConfig: configParams, } - if svc.updatingMapName != "" { - req.MapName = svc.updatingMapName - req.ExistingMapVersion = svc.updatingMapVersion + if svc.updatingMap != nil { + req.MapName = svc.updatingMap.name + req.ExistingMapVersion = svc.updatingMap.version updatingMode = true } resp, err := svc.app.CSClient.StartMappingSession(ctx, req) From 6d5fc3d65f822465b6e5d4e64e42ccff08a938b4 Mon Sep 17 00:00:00 2001 From: John Nicholson Date: Fri, 20 Mar 2026 13:57:32 -0400 Subject: [PATCH 6/6] require machine_part_id at startup part_id is now required (from config or env var), matching the existing machine_id behavior. removes the dead partID != "" guard in initialize. Co-Authored-By: Claude Sonnet 4.6 --- cloudslam/cloudslam.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cloudslam/cloudslam.go b/cloudslam/cloudslam.go index e15e61e..9621a3a 100644 --- a/cloudslam/cloudslam.go +++ b/cloudslam/cloudslam.go @@ -174,6 +174,9 @@ func newSLAM( if partID == "" { partID = os.Getenv(rdkutils.MachinePartIDEnvVar) } + if partID == "" { + return nil, fmt.Errorf("machine_part_id is required but not set in config or %s env var", rdkutils.MachinePartIDEnvVar) + } viamVersion := newConf.VIAMVersion if viamVersion == "" { @@ -248,11 +251,11 @@ func (svc *cloudslamWrapper) initialize(mappingMode slam.MappingMode) error { return err } - // only attempt updating mode if a partID is configured and the user is not trying to make a new map. + // only attempt updating mode if the user is not trying to make a new map. // the cloudslam can still make a new map if no slam_map package is found. // the webapp does not remove the package from the config when swapping from updating mode to mapping mode, so this code // needs the extra check to ensure we only update maps when the user wants to. - if svc.partID != "" && mappingMode != slam.MappingModeNewMap { + if mappingMode != slam.MappingModeNewMap { name, version, err := svc.app.GetSLAMMapPackageOnRobot(svc.cancelCtx, svc.partID) if err != nil { return err