Skip to content

Fix PTZ bugs with unsupported & unchanged fields - #3192

Open
samadDotDev wants to merge 1 commit into
mainfrom
fix/ptz-issues
Open

Fix PTZ bugs with unsupported & unchanged fields#3192
samadDotDev wants to merge 1 commit into
mainfrom
fix/ptz-issues

Conversation

@samadDotDev

Copy link
Copy Markdown
Contributor

Description of Change

Issue # 1

The attribute handler and device config used MPAN/MTILT/MZOOM/MPRESETS (based on "feature code" from spec), which don't exist in the generated files in lua libs (CameraAvSettingsUserLevelManagement/types/Feature.lua only defines MECHANICAL_PAN/TILT/ZOOM/PRESETS based on "feature name" in SDK XML).

In Lua, indexing an undefined table field returns nil rather than erroring, so every feature-support check silently evaluated to "supported," and MatterDevice:get_endpoints's opts.feature_bitmap == nil short-circuit treated any device with the cluster as having all PTZ features. On a device whose FeatureMap excludes zoom, this caused the driver to read a zoom field that the device never sends, crashing with attempt to index a nil value (field 'zoom').

Fixed by using the correct MECHANICAL_* names everywhere.

Issue # 2

Separately, ptz_relative_move_factory always sent all three deltas to MPTZRelativeMove, defaulting unused axes to 0 instead of omitting them. Per spec, each delta's conformance is tied to its own feature bit, so an explicit 0 isn't the same as an omitted field and essentially a zoom-less device rejected the command with INVALID_COMMAND.

Fixed by only setting the delta for the axis being moved and leaving the others nil.

Summary of Completed Tests

  • Tested with the same camera that doesn't support zoom in PTZ (has Pan-Tilt-only feature)
  • Tested with a different camera that supports PTZ
  • Added a unit test covering the first fix with a new mock device fixture (that doesn't support zoom), and updated existing tests for the second fix (omitting unchanged/unsupported fields instead of defaulting to 0)

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

Test Results

   73 files    541 suites   0s ⏱️
3 259 tests 3 259 ✅ 0 💤 0 ❌
5 248 runs  5 248 ✅ 0 💤 0 ❌

Results for commit 80a3dce.

@github-actions

Copy link
Copy Markdown

matter-switch_coverage.xml

File Coverage
All files 92%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/init.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/third_reality_garage_door/init.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/third_reality_mk1/init.lua 93%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/ikea_scroll/scroll_utils/event_utils.lua 52%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/init.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua 96%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_handlers/capability_handlers.lua 83%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_handlers/attribute_handlers.lua 81%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_handlers/event_handlers.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_handlers/capability_handlers.lua 92%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/aqara_cube/init.lua 95%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_utils/device_configuration.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/switch_utils/utils.lua 92%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/ikea_scroll/scroll_handlers/event_handlers.lua 75%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/hager/init.lua 93%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/ikea_scroll/init.lua 90%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/device_configuration.lua 94%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/utils.lua 98%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/eve_energy/init.lua 91%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 80a3dce

@ldeora

ldeora commented Aug 22, 2026

Copy link
Copy Markdown

One related PTZ issue remains in ptz_set_position_factory() and is tracked in #3184.

Single-axis commands still depend on the cached values of the other axes:

setPan(20)
    → MPTZSetPosition(20, cachedTilt, cachedZoom)

For Matter's optional PTZ fields, this should instead be:

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

This is not only relevant for cameras without one of the mechanical features. On the G350, all three axes are supported, but we observed a valid state where pan and tilt were populated while the current zoom value was temporarily null.

So a setPan or setTilt command should not require unrelated cached axis state to be available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants