Skip to content

Matter camera: single-axis MPTZSetPosition commands should not depend on cached values for the other axes #3184

Description

@ldeora

Summary

The current mechanicalPanTiltZoom handler implements SmartThings setPan, setTilt and setZoom by loading the cached state of all three axes and then sending all three values in MPTZSetPosition.

This is unsafe because a camera can support an axis while its current SmartThings state is temporarily unknown.

A real G350 state demonstrated exactly this:

pan  = -36
tilt = 15
zoom = null

while mechanical zoom itself was supported and a valid zoom range was reported.

For single-axis commands, the driver should send only the axis requested by the SmartThings command and leave the other optional MPTZSetPosition fields omitted.

Environment / verification

Verified against SmartThingsCommunity/SmartThingsEdgeDrivers main at commit
60bbf2716412fdf04fd59f967bdd37be6995d664 (2026-08-12).

Real-device validation used:

  • Aqara Camera Hub G350
  • VID/PID: 0x115F / 0x3013
  • firmware: 4.5.70 / 4005070
  • reported Matter SpecificationVersion: 1.5.1
  • Camera device type: 0x0142
  • Camera endpoint in this device: endpoint 2

The proposed changes are generic Matter-camera changes. No Aqara VID/PID fingerprint or proprietary Aqara cluster handling is required.

Current behavior

The current handler:

  1. obtains cached pan/tilt/zoom;
  2. changes only the requested value;
  3. clamps all cached values;
  4. sends all three values.

See:

Conceptually:

setPan:
  current.pan = requested_pan

for every axis:
  current = clamp(current)

MPTZSetPosition(current.pan, current.tilt, current.zoom)

If an unrelated cached axis is nil, a valid single-axis command unnecessarily depends on unavailable state.

Reproduction

  1. Use a mechanical PTZ camera.
  2. Have valid pan and tilt state but no current zoom state.
  3. Execute SmartThings setPan.
  4. The current code still attempts to clamp and send the zoom value.

The G350 has been observed in exactly this state:

"mechanicalPanTiltZoom": {
  "pan": { "value": -36 },
  "tilt": { "value": 15 },
  "zoom": { "value": null },
  "zoomRange": {
    "value": {
      "minimum": 1,
      "maximum": 81
    }
  }
}

Expected behavior

Single-axis commands should not require unrelated current-axis values.

Suggested mapping:

setPan         -> MPTZSetPosition(pan,  nil,  nil)
setTilt        -> MPTZSetPosition(nil,  tilt, nil)
setZoom        -> MPTZSetPosition(nil,  nil,  zoom)
setPanTiltZoom -> MPTZSetPosition(pan,  tilt, zoom)

Clamp only values actually supplied by the SmartThings command.

Related PTZ completeness

The G350 also advertises the standard MovementState attribute, but the current driver does not subscribe to or retain it.

Current mechanical PTZ subscription:

Current AV Settings handlers:

MovementState can be subscribed and retained internally even if the current SmartThings capability has no user-facing attribute for it.

Also, dynamic profiling currently enables mechanicalPanTiltZoom when pan, tilt or zoom is present, but not when the camera supports only mechanical presets. Including the MECHANICAL_PRESETS feature in that decision is a small consistency improvement.

Suggested code changes

  1. Update ptz_set_position_factory() to construct only the requested optional coordinates.
  2. Keep setPanTiltZoom as the all-three-axis command.
  3. Subscribe to CameraAvSettingsUserLevelManagement.MovementState.
  4. Cache MovementState internally.
  5. Include MECHANICAL_PRESETS in the mechanical PTZ capability-profile decision.
  6. If AcceptedCommandList support is added separately, gate MPTZ commands against it.

G350 evidence

The G350 advertises:

MechanicalPan
MechanicalTilt
MechanicalZoom
MechanicalPresets

and these accepted commands:

MPTZSetPosition
MPTZRelativeMove
MPTZMoveToPreset
MPTZSavePreset
MPTZRemovePreset

The real SmartThings state showed a supported but temporarily null zoom value while pan/tilt were populated, demonstrating that the current all-axis assumption is not safe.

The optional-coordinate implementation was included in the tested camera build without introducing startup/runtime regressions. Dedicated PTZ command testing should still be retained as regression coverage.

Affected files

  • src/sub_drivers/camera/camera_handlers/capability_handlers.lua
  • src/sub_drivers/camera/camera_handlers/attribute_handlers.lua
  • src/sub_drivers/camera/camera_utils/device_configuration.lua
  • src/sub_drivers/camera/camera_utils/utils.lua
  • src/sub_drivers/camera/camera_utils/fields.lua
  • src/sub_drivers/camera/init.lua
  • src/test/test_matter_camera.lua

Suggested regression coverage

  1. setPan works when cached zoom is nil.
  2. setTilt works when cached pan is nil.
  3. setZoom works when cached tilt is nil.
  4. Only the requested optional coordinate is present in single-axis MPTZSetPosition.
  5. setPanTiltZoom still sends all three.
  6. MovementState reports do not cause errors and are retained.
  7. a presets-only mechanical feature combination can still enable the PTZ capability if supported by the SmartThings capability model.

Minimal suggested diff

diff --git a/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua b/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua
index 53b3571..6769376 100644
--- a/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua
+++ b/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua
@@ -311,6 +311,10 @@ function CameraAttributeHandlers.ptz_position_handler(driver, device, ib, respon
   end
 end
 
+function CameraAttributeHandlers.movement_state_handler(driver, device, ib, response)
+  device:set_field(camera_fields.MOVEMENT_STATE, ib.data.value)
+end
+
 function CameraAttributeHandlers.ptz_presets_handler(driver, device, ib, response)
   if not ib.data.elements then return end
   local presets = {}
diff --git a/src/sub_drivers/camera/camera_handlers/capability_handlers.lua b/src/sub_drivers/camera/camera_handlers/capability_handlers.lua
index a26afd0..197a10b 100644
--- a/src/sub_drivers/camera/camera_handlers/capability_handlers.lua
+++ b/src/sub_drivers/camera/camera_handlers/capability_handlers.lua
@@ -151,23 +151,23 @@ end
 CameraCapabilityHandlers.ptz_set_position_factory = function(command)
   return function (driver, device, cmd)
     local ptz_map = camera_utils.get_ptz_map(device)
+    -- MPTZSetPosition coordinates are optional. Single-axis commands should not
+    -- depend on cached values for the other two axes.
+    local pan, tilt, zoom
     if command == capabilities.mechanicalPanTiltZoom.commands.setPanTiltZoom then
-      ptz_map[camera_fields.PAN_IDX].current = cmd.args.pan
-      ptz_map[camera_fields.TILT_IDX].current = cmd.args.tilt
-      ptz_map[camera_fields.ZOOM_IDX].current = cmd.args.zoom
+      pan = utils.clamp_value(cmd.args.pan, ptz_map[camera_fields.PAN_IDX].range.minimum, ptz_map[camera_fields.PAN_IDX].range.maximum)
+      tilt = utils.clamp_value(cmd.args.tilt, ptz_map[camera_fields.TILT_IDX].range.minimum, ptz_map[camera_fields.TILT_IDX].range.maximum)
+      zoom = utils.clamp_value(cmd.args.zoom, ptz_map[camera_fields.ZOOM_IDX].range.minimum, ptz_map[camera_fields.ZOOM_IDX].range.maximum)
     elseif command == capabilities.mechanicalPanTiltZoom.commands.setPan then
-      ptz_map[camera_fields.PAN_IDX].current = cmd.args.pan
+      pan = utils.clamp_value(cmd.args.pan, ptz_map[camera_fields.PAN_IDX].range.minimum, ptz_map[camera_fields.PAN_IDX].range.maximum)
     elseif command == capabilities.mechanicalPanTiltZoom.commands.setTilt then
-      ptz_map[camera_fields.TILT_IDX].current = cmd.args.tilt
+      tilt = utils.clamp_value(cmd.args.tilt, ptz_map[camera_fields.TILT_IDX].range.minimum, ptz_map[camera_fields.TILT_IDX].range.maximum)
     else
-      ptz_map[camera_fields.ZOOM_IDX].current = cmd.args.zoom
-    end
-    for _, v in pairs(ptz_map) do
-      v.current = utils.clamp_value(v.current, v.range.minimum, v.range.maximum)
+      zoom = utils.clamp_value(cmd.args.zoom, ptz_map[camera_fields.ZOOM_IDX].range.minimum, ptz_map[camera_fields.ZOOM_IDX].range.maximum)
     end
     local endpoint_id = device:component_to_endpoint(cmd.component)
-    device:send(clusters.CameraAvSettingsUserLevelManagement.server.commands.MPTZSetPosition(device, endpoint_id,
-      ptz_map[camera_fields.PAN_IDX].current, ptz_map[camera_fields.TILT_IDX].current, ptz_map[camera_fields.ZOOM_IDX].current
+    device:send(clusters.CameraAvSettingsUserLevelManagement.server.commands.MPTZSetPosition(
+      device, endpoint_id, pan, tilt, zoom
     ))
   end
 end
diff --git a/src/sub_drivers/camera/camera_utils/device_configuration.lua b/src/sub_drivers/camera/camera_utils/device_configuration.lua
index 0619194..3d1cc00 100644
--- a/src/sub_drivers/camera/camera_utils/device_configuration.lua
+++ b/src/sub_drivers/camera/camera_utils/device_configuration.lua
@@ -256,7 +256,8 @@ function CameraDeviceConfiguration.match_profile(device)
         end
         if clus_has_feature(clusters.CameraAvSettingsUserLevelManagement.types.Feature.MECHANICAL_PAN) or
           clus_has_feature(clusters.CameraAvSettingsUserLevelManagement.types.Feature.MECHANICAL_TILT) or
-          clus_has_feature(clusters.CameraAvSettingsUserLevelManagement.types.Feature.MECHANICAL_ZOOM) then
+          clus_has_feature(clusters.CameraAvSettingsUserLevelManagement.types.Feature.MECHANICAL_ZOOM) or
+          clus_has_feature(clusters.CameraAvSettingsUserLevelManagement.types.Feature.MECHANICAL_PRESETS) then
           table.insert(main_component_capabilities, capabilities.mechanicalPanTiltZoom.ID)
         end
       elseif ep_cluster.cluster_id == clusters.ZoneManagement.ID and has_server_cluster_type(ep_cluster) and
diff --git a/src/sub_drivers/camera/camera_utils/fields.lua b/src/sub_drivers/camera/camera_utils/fields.lua
index c88f177..5eb763e 100644
--- a/src/sub_drivers/camera/camera_utils/fields.lua
+++ b/src/sub_drivers/camera/camera_utils/fields.lua
@@ -16,6 +16,7 @@ CameraFields.TRIGGERED_ZONES = "__triggered_zones"
 CameraFields.DPTZ_VIEWPORTS = "__dptz_viewports"
 CameraFields.STATUS_LIGHT_ENABLED_PRESENT = "__status_light_enabled_present"
 CameraFields.STATUS_LIGHT_BRIGHTNESS_PRESENT = "__status_light_brightness_present"
+CameraFields.MOVEMENT_STATE = "__camera_movement_state"
 
 CameraFields.CameraAVSMFeatureMapAttr = { ID = 0xFFFC, cluster = clusters.CameraAvStreamManagement.ID }
 CameraFields.CameraAVSULMFeatureMapAttr = { ID = 0xFFFC, cluster = clusters.CameraAvSettingsUserLevelManagement.ID }
diff --git a/src/sub_drivers/camera/camera_utils/utils.lua b/src/sub_drivers/camera/camera_utils/utils.lua
index 78792b9..298760d 100644
--- a/src/sub_drivers/camera/camera_utils/utils.lua
+++ b/src/sub_drivers/camera/camera_utils/utils.lua
@@ -210,7 +210,8 @@ function CameraUtils.subscribe(device)
       clusters.CameraAvSettingsUserLevelManagement.attributes.PanMax,
       clusters.CameraAvSettingsUserLevelManagement.attributes.PanMin,
       clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMax,
-      clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMin
+      clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMin,
+      clusters.CameraAvSettingsUserLevelManagement.attributes.MovementState
     },
     [capabilities.audioMute.ID] = {
       clusters.CameraAvStreamManagement.attributes.SpeakerMuted,
diff --git a/src/sub_drivers/camera/init.lua b/src/sub_drivers/camera/init.lua
index 2aa698a..6a1d860 100644
--- a/src/sub_drivers/camera/init.lua
+++ b/src/sub_drivers/camera/init.lua
@@ -112,6 +112,7 @@ local camera_handler = {
         [clusters.CameraAvSettingsUserLevelManagement.attributes.PanMin.ID] = attribute_handlers.pt_range_handler_factory(capabilities.mechanicalPanTiltZoom.panRange, camera_fields.pt_range_fields[camera_fields.PAN_IDX].min),
         [clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMax.ID] = attribute_handlers.pt_range_handler_factory(capabilities.mechanicalPanTiltZoom.tiltRange, camera_fields.pt_range_fields[camera_fields.TILT_IDX].max),
         [clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMin.ID] = attribute_handlers.pt_range_handler_factory(capabilities.mechanicalPanTiltZoom.tiltRange, camera_fields.pt_range_fields[camera_fields.TILT_IDX].min),
+        [clusters.CameraAvSettingsUserLevelManagement.attributes.MovementState.ID] = attribute_handlers.movement_state_handler,
         [clusters.CameraAvSettingsUserLevelManagement.attributes.DPTZStreams.ID] = attribute_handlers.dptz_streams_handler,
         [camera_fields.CameraAVSULMFeatureMapAttr.ID] = attribute_handlers.camera_feature_map_handler
       },

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions