Maintenance Window - Block Reconfigure on all errors when reading a s…

…ensor (viamrobotics#4509)
jckras · Oct 31, 2024 · a34352b · a34352b
1 parent 5b26371
commit a34352b
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 13 deletions.
diff --git a/robot/impl/local_robot.go b/robot/impl/local_robot.go
@@ -1207,7 +1207,11 @@ func (r *localRobot) reconfigure(ctx context.Context, newConfig *config.Config,
 			} else {
 				canReconfigure, err := r.checkMaintenanceSensorReadings(ctx, newConfig.MaintenanceConfig.MaintenanceAllowedKey, sensorComponent)
 				if !canReconfigure {
-					r.logger.Info("maintenance_allowed_key found from readings on maintenance sensor. Skipping reconfiguration.")
+					if err != nil {
+						r.logger.CErrorw(ctx, "error reading maintenance sensor", "error", err)
+					} else {
+						r.logger.Info("maintenance_allowed_key found from readings on maintenance sensor. Skipping reconfiguration.")
+					}
 					diff, err := config.DiffConfigs(*r.Config(), *newConfig, false)
 					if err != nil {
 						r.logger.CErrorw(ctx, "error diffing the configs", "error", err)
@@ -1218,11 +1222,7 @@ func (r *localRobot) reconfigure(ctx context.Context, newConfig *config.Config,
 					}
 					return
 				}
-				if err != nil {
-					r.logger.Warn(err.Error() + ". Starting reconfiguration")
-				} else {
-					r.logger.Info("maintenance_allowed_key found from readings on maintenance sensor. Starting reconfiguration")
-				}
+				r.logger.Info("maintenance_allowed_key found from readings on maintenance sensor. Starting reconfiguration")
 			}
 		}
 	}
@@ -1528,11 +1528,11 @@ func (r *localRobot) checkMaintenanceSensorReadings(ctx context.Context,
 	}
 	readingVal, ok := readings[maintenanceAllowedKey]
 	if !ok {
-		return true, errors.Errorf("error getting maintenance_allowed_key %s from sensor reading", maintenanceAllowedKey)
+		return false, errors.Errorf("error getting maintenance_allowed_key %s from sensor reading", maintenanceAllowedKey)
 	}
 	canReconfigure, ok := readingVal.(bool)
 	if !ok {
-		return true, errors.Errorf("maintenance_allowed_key %s is not a bool value", maintenanceAllowedKey)
+		return false, errors.Errorf("maintenance_allowed_key %s is not a bool value", maintenanceAllowedKey)
 	}
 	return canReconfigure, nil
 }
diff --git a/robot/impl/local_robot_test.go b/robot/impl/local_robot_test.go
@@ -3969,31 +3969,31 @@ func TestCheckMaintenanceSensorReadings(t *testing.T) {
 		localRobot := r.(*localRobot)
 		canReconfigure, err := localRobot.checkMaintenanceSensorReadings(context.Background(), "keyDoesNotExist", newValidSensor())
 
-		test.That(t, canReconfigure, test.ShouldEqual, true)
+		test.That(t, canReconfigure, test.ShouldEqual, false)
 		test.That(t, err.Error(), test.ShouldEqual, "error getting maintenance_allowed_key keyDoesNotExist from sensor reading")
 	})
 	t.Run("maintenanceAllowedKey is a number not a boolean", func(t *testing.T) {
 		r := setupLocalRobot(t, context.Background(), &config.Config{}, logger)
 		localRobot := r.(*localRobot)
 		canReconfigure, err := localRobot.checkMaintenanceSensorReadings(context.Background(), "ThatIsNotAWallet", newValidSensor())
 
-		test.That(t, canReconfigure, test.ShouldEqual, true)
+		test.That(t, canReconfigure, test.ShouldEqual, false)
 		test.That(t, err.Error(), test.ShouldEqual, "maintenance_allowed_key ThatIsNotAWallet is not a bool value")
 	})
 	t.Run("maintenanceAllowedKey is one not a boolean", func(t *testing.T) {
 		r := setupLocalRobot(t, context.Background(), &config.Config{}, logger)
 		localRobot := r.(*localRobot)
 		canReconfigure, err := localRobot.checkMaintenanceSensorReadings(context.Background(), "OneIsNotTrue", newValidSensor())
 
-		test.That(t, canReconfigure, test.ShouldEqual, true)
+		test.That(t, canReconfigure, test.ShouldEqual, false)
 		test.That(t, err.Error(), test.ShouldEqual, "maintenance_allowed_key OneIsNotTrue is not a bool value")
 	})
 	t.Run("maintenanceAllowedKey is string true not a boolean", func(t *testing.T) {
 		r := setupLocalRobot(t, context.Background(), &config.Config{}, logger)
 		localRobot := r.(*localRobot)
 		canReconfigure, err := localRobot.checkMaintenanceSensorReadings(context.Background(), "TrueIsNotTrue", newValidSensor())
 
-		test.That(t, canReconfigure, test.ShouldEqual, true)
+		test.That(t, canReconfigure, test.ShouldEqual, false)
 		test.That(t, err.Error(), test.ShouldEqual, "maintenance_allowed_key TrueIsNotTrue is not a bool value")
 	})
 }
@@ -4058,7 +4058,7 @@ func newErrorSensor() sensor.Sensor {
 func newInvalidSensor() sensor.Sensor {
 	s := &inject.Sensor{}
 	s.ReadingsFunc = func(ctx context.Context, extra map[string]interface{}) (map[string]interface{}, error) {
-		return map[string]any{"ThatsMyWallet": 1, "ThatsNotMyWallet": 2}, nil
+		return map[string]any{"ThatsMyWallet": true, "ThatsNotMyWallet": false}, nil
 	}
 	return s
 }