auto-populate machine IDs from env vars and validate sensor config

README.md

@@ -17,11 +17,12 @@ On the new service panel, copy and paste the following attribute template into y
    "api_key": "<location-api-key>",
    "api_key_id": "<location-api-key-id>",
    "organization_id": "<organization_id>",
-   "location_id": "<location_id>",
-   "machine_id": "<machine_id>",
+   "location_id": "<location_id>"
 }
 ```
 
+> **Note:** `machine_id` and `machine_part_id` are automatically read from the `VIAM_MACHINE_ID` and `VIAM_MACHINE_PART_ID` environment variables that viam-server sets on every machine. You only need to set them explicitly in the config when running outside of viam-server.
+
 In addition, in your Cartographer config the setting `"use_cloud_slam"` must be set to `true`. This only applies when trying to use cloudslam. Uploading a locally built map does not require this setting.
 
 ### Attributes
@@ -35,8 +36,8 @@ The following attributes are available for `viam:cloudslam-wrapper:cloudslam`
 | `api_key_id` | string | **Required**     | location owner API key id        |
 | `organization_id` | string | **Required**     | id string for your [organization](https://docs.viam.com/cloud/organizations/)        |
 | `location_id` | string | **Required**     | id string for your [location](https://docs.viam.com/cloud/locations/)        |
-| `machine_id` | string | **Required**     | id string for your [machine](https://docs.viam.com/appendix/apis/fleet/#find-machine-id)        |
-| `machine_part_id` | string | Optional     | optional id string for the [machine part](https://docs.viam.com/appendix/apis/fleet/#find-machine-id). Used for local package creation and updating mode       |
+| `machine_id` | string | Optional     | id string for your [machine](https://docs.viam.com/appendix/apis/fleet/#find-machine-id). Defaults to the `VIAM_MACHINE_ID` env var set by viam-server        |
+| `machine_part_id` | string | Optional     | id string for the [machine part](https://docs.viam.com/appendix/apis/fleet/#find-machine-id). Defaults to the `VIAM_MACHINE_PART_ID` env var set by viam-server. Used for data capture validation, local package creation, and updating mode       |
 | `viam_version` | string | Optional     | optional string to identify which version of viam-server to use with cloudslam. Defaults to `stable`        |
 | `slam_version` | string | Optional     | optional string to identify which version of cartographer to use with cloudslam. Defaults to `stable`         |
 | `camera_freq_hz` | float | Optional     | set the expected capture frequency for your camera/lidar components. Defaults to `5`        |
@@ -47,16 +48,14 @@ The following attributes are available for `viam:cloudslam-wrapper:cloudslam`
 ```json
 {
   "slam_service": "my-actual-slam-service",
-   "api_key": "location-api-key",
-   "api_key_id": "location-api-key-id",
-   "organization_id": "organization_id",
-   "location_id": "location_id",
-   "machine_id": "machine_id",
-   "machine_part_id": "machine_part_id", 
-   "camera_freq_hz": 5.0,
-   "movement_sensor_freq_hz": 20.0, 
-   "slam_version": "stable", 
-   "viam_version": "stable", 
+  "api_key": "location-api-key",
+  "api_key_id": "location-api-key-id",
+  "organization_id": "organization_id",
+  "location_id": "location_id",
+  "camera_freq_hz": 5.0,
+  "movement_sensor_freq_hz": 20.0,
+  "slam_version": "stable",
+  "viam_version": "stable"
 }
 ```
 
@@ -110,4 +109,4 @@ To interact with a cloudslam mapping session, go to the `DoCommand` on the [Cont
 - {`"stop": ""`} will stop an active cloudslam mapping session if one is running. The completed map can be found on the SLAM library tab of the machines page
 - {`"save-local-map": "<MAP_NAME>"`} will grab the current map from the configured SLAM service and upload it to your location, in the SLAM library tab of the machines page
 
-For updating a map using cloudslam, a `machine_part_id` must be configured. When configured, the module will check the machine's config to see if any slam maps are configured on the robot. If a slam map is found, cloudslam will be configured for updating mode and the map name will be inherited from the configured map.
+For updating a map using cloudslam, the module checks the machine's config for any slam map packages. If one is found and the SLAM service is not in new-map mode, cloudslam will start in updating mode and inherit the map name from the configured package.

cloudslam/app.go

@@ -3,6 +3,7 @@ package cloudslam
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -12,6 +13,7 @@ import (
 	pbPackage "go.viam.com/api/app/packages/v1"
 	pbApp "go.viam.com/api/app/v1"
 	"go.viam.com/rdk/logging"
+	"go.viam.com/rdk/services/slam"
 	"go.viam.com/utils/rpc"
 )
 
@@ -101,6 +103,79 @@ func (app *AppClient) GetDataFromHTTP(ctx context.Context, dataURL string) ([]by
 	return io.ReadAll(res.Body)
 }
 
+// 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 {
+	req := pbApp.ConfigRequest{Id: partID}
+	resp, err := app.RobotClient.Config(ctx, &req)
+	if err != nil {
+		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{}{}
+	}
+
+	sensorTypes := make(map[string]slam.SensorType, len(sensors))
+	for _, s := range sensors {
+		sensorTypes[s.name] = s.sensorType
+	}
+
+	for _, comp := range resp.GetConfig().GetComponents() {
+		if _, ok := pending[comp.GetName()]; !ok {
+			continue
+		}
+		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())
+		}
+		if hasEnabledDataCapture(comp, sensorTypes[comp.GetName()]) {
+			delete(pending, comp.GetName())
+		}
+	}
+
+	if len(pending) > 0 {
+		missing := make([]string, 0, len(pending))
+		for name := range pending {
+			missing = append(missing, name)
+		}
+		return fmt.Errorf("the following sensors do not have data capture enabled: %v", missing)
+	}
+	return nil
+}
+
+// hasEnabledDataCapture returns true if the component has an appropriate enabled capture method
+// in its data_manager service config. For cameras, NextPointCloud must be configured and enabled.
+// For other sensor types, any enabled capture method is sufficient.
+func hasEnabledDataCapture(comp *pbApp.ComponentConfig, sensorType slam.SensorType) bool {
+	for _, svcConfig := range comp.GetServiceConfigs() {
+		if svcConfig.GetType() != "rdk:service:data_manager" {
+			continue
+		}
+		captureMethods := svcConfig.GetAttributes().GetFields()["capture_methods"]
+		if captureMethods == nil {
+			continue
+		}
+		for _, method := range captureMethods.GetListValue().GetValues() {
+			fields := method.GetStructValue().GetFields()
+			if fields["disabled"].GetBoolValue() {
+				continue
+			}
+			if sensorType == slam.SensorTypeCamera {
+				if fields["method"].GetStringValue() == "NextPointCloud" {
+					return true
+				}
+			} else {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // Close closes the app clients.
 func (app *AppClient) Close() error {
 	// close any idle connections to prevent goleaks. Possibly redundant with DisableKeepAlives

cloudslam/cloudslam.go

@@ -12,12 +12,15 @@ import (
 	"sync/atomic"
 	"time"
 
+	"os"
+
 	pbCloudSLAM "go.viam.com/api/app/cloudslam/v1"
 	"go.viam.com/rdk/grpc"
 	"go.viam.com/rdk/logging"
 	"go.viam.com/rdk/resource"
 	"go.viam.com/rdk/services/slam"
 	"go.viam.com/rdk/spatialmath"
+	rdkutils "go.viam.com/rdk/utils"
 	goutils "go.viam.com/utils"
 	"google.golang.org/protobuf/types/known/structpb"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -122,9 +125,6 @@ func (cfg *Config) Validate(path string) ([]string, []string, error) {
 	if cfg.APIKeyID == "" {
 		return []string{}, []string{}, resource.NewConfigValidationFieldRequiredError(path, "api_key_id")
 	}
-	if cfg.MachineID == "" {
-		return []string{}, []string{}, resource.NewConfigValidationFieldRequiredError(path, "machine_id")
-	}
 	if cfg.LocationID == "" {
 		return []string{}, []string{}, resource.NewConfigValidationFieldRequiredError(path, "location_id")
 	}
@@ -149,6 +149,19 @@ func newSLAM(
 		return nil, err
 	}
 
+	machineID := newConf.MachineID
+	if machineID == "" {
+		machineID = os.Getenv(rdkutils.MachineIDEnvVar)
+	}
+	if machineID == "" {
+		return nil, fmt.Errorf("machine_id is required but not set in config or %s env var", rdkutils.MachineIDEnvVar)
+	}
+
+	partID := newConf.PartID
+	if partID == "" {
+		partID = os.Getenv(rdkutils.MachinePartIDEnvVar)
+	}
+
 	viamVersion := newConf.VIAMVersion
 	if viamVersion == "" {
 		viamVersion = "stable"
@@ -192,10 +205,10 @@ func newSLAM(
 		logger:         logger,
 		cancelCtx:      cancelCtx,
 		cancelFunc:     cancel,
-		machineID:      newConf.MachineID,
+		machineID:      machineID,
 		locationID:     newConf.LocationID,
 		organizationID: newConf.OrganizationID,
-		partID:         newConf.PartID,
+		partID:         partID,
 		sensors:        csSensors,
 		app:            appClients,
 	}
@@ -317,6 +330,11 @@ 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
+			}
+		}
 		jobID, isUpdating, err := svc.StartJob(svc.cancelCtx, name.(string))
 		if err != nil {
 			return nil, err
@@ -361,6 +379,15 @@ func (svc *cloudslamWrapper) StopJob(ctx context.Context) (string, error) {
 	if err != nil {
 		return "", err
 	}
+
+	metaResp, err := svc.app.CSClient.GetMappingSessionMetadataByID(ctx,
+		&pbCloudSLAM.GetMappingSessionMetadataByIDRequest{SessionId: currJob})
+	if err != nil {
+		svc.logger.Warnf("could not retrieve session metadata for job %s: %v", currJob, err)
+	} else if metaResp.GetSessionMetadata().GetEndStatus() == pbCloudSLAM.EndStatus_END_STATUS_FAIL {
+		return "", fmt.Errorf("cloudslam session failed: %s", metaResp.GetSessionMetadata().GetErrorMsg())
+	}
+
 	packageName := strings.Split(resp.GetPackageId(), "/")[1]
 	packageURL := svc.app.baseURL + "/robots?name=" + packageName + "&version=" + resp.GetVersion()
 	return packageURL, nil