Support additional Camera device types#2590
Open
nickolas-deboom wants to merge 2 commits into
Open
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 42bfb6a |
samadDotDev
approved these changes
Feb 10, 2026
Contributor
samadDotDev
left a comment
samadDotDev
left a comment
There was a problem hiding this comment.
Will need to re-evaluate after rebase
hcarter-775
reviewed
Feb 10, 2026
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
from
96ccae6 to
67332b9
Compare
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
2 times, most recently
from
e012e2a to
a25f0c7
Compare
hcarter-775
reviewed
Mar 3, 2026
hcarter-775
reviewed
Mar 3, 2026
hcarter-775
reviewed
Mar 3, 2026
hcarter-775
reviewed
Mar 3, 2026
hcarter-775
reviewed
Mar 3, 2026
nickolas-deboom
commented
Mar 5, 2026
| @@ -6,116 +6,166 @@ local dkjson = require "dkjson" | |||
| local clusters = require "st.matter.clusters" | |||
|
|
|||
| local mock_device = test.mock_device.build_test_matter_device({ | |||
Contributor
Author
There was a problem hiding this comment.
Mentioned elsewhere, but after making the changes in 0542fb3 the tests here began to fail because they expect subscriptions to the button events yet no endpoints with the Switch cluster were present in the mock device. I added the endpoints and also fixed the tabbing in this file which is why the diff looks so large
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
2 times, most recently
from
31bd85e to
164095f
Compare
samadDotDev
approved these changes
Mar 5, 2026
samadDotDev
reviewed
Mar 6, 2026
| if status_light_brightness_present then | ||
| table.insert(status_led_component_capabilities, capabilities.mode.ID) | ||
| end | ||
| if #device:get_endpoints(clusters.Chime.ID, {cluster_type = "SERVER"}) > 0 then |
Contributor
There was a problem hiding this comment.
SERVER or BOTH? Basically reuse logic similar to has_server_cluster_type()
Contributor
Author
There was a problem hiding this comment.
Done in b082796, does that look alright? It uses the first endpoint containing the Chime cluster, but I wasn't sure if the client and server clusters would necessarily be on the same endpoint or not
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
2 times, most recently
from
b1faa74 to
297c2e3
Compare
hcarter-775
reviewed
Mar 9, 2026
| --- @param subscribed_events table key-value pairs mapping capability ids to subscribed events | ||
| function utils.populate_subscribe_request_for_device(checked_device, subscribe_request, capabilities_seen, attributes_seen, events_seen, subscribed_attributes, subscribed_events) | ||
| for _, component in pairs(checked_device.st_store.profile.components) do | ||
| function utils.populate_subscribe_request_for_device(checked_device, parent_device, subscribe_request, capabilities_seen, attributes_seen, events_seen, subscribed_attributes, subscribed_events) |
Contributor
There was a problem hiding this comment.
Before merging, let's document a little testing here, since this change does affect our subscription logic. LGTM, but I think it's still worth ensuring we haven't missed something
hcarter-775
reviewed
Mar 9, 2026
hcarter-775
reviewed
Mar 9, 2026
hcarter-775
reviewed
Mar 9, 2026
hcarter-775
reviewed
Mar 9, 2026
samadDotDev
reviewed
Mar 10, 2026
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
2 times, most recently
from
63e0a39 to
17d5bb3
Compare
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
from
a19ba8c to
0080716
Compare
ctowns
reviewed
Jun 3, 2026
| local version = require "version" | ||
| if version.rpc >= 10 and version.api >= 16 and | ||
| #switch_utils.get_endpoints_by_device_type(device, fields.DEVICE_TYPE_ID.CAMERA) > 0 then | ||
| #device:get_endpoints(clusters.CameraAvStreamManagement.ID, {cluster_type = "SERVER"}) > 0 or |
Contributor
There was a problem hiding this comment.
We would want to use the version gating for any of these device types, so you'd need to add parentesis around the string of or conditionals so that the logic is something like if version > X and (has_camera_cluster or is_chime or is_doorbell):
if version.rpc >= 10 and version.api >= 16 and
(#device:get_endpoints(clusters.CameraAvStreamManagement.ID, { cluster_type = "SERVER" }) > 0 or
#switch_utils.get_endpoints_by_device_type(device, fields.DEVICE_TYPE_ID.CHIME) > 0 or
#switch_utils.get_endpoints_by_device_type(device, fields.DEVICE_TYPE_ID.DOORBELL) > 0) then
return true, require("sub_drivers.camera")
end
ctowns
reviewed
Jun 3, 2026
| table.insert(main_component_capabilities, capabilities.nightVision.ID) | ||
| end | ||
| if #chime_endpoints > 0 then | ||
| table.insert(main_component_capabilities, capabilities.sounds.ID) |
Contributor
There was a problem hiding this comment.
We would also need to include audioMute for handling the Enabled attribute on the chime cluster, which is mandatory
ctowns
reviewed
Jun 3, 2026
| --- @param subscribed_events table key-value pairs mapping capability ids to subscribed events | ||
| function utils.populate_subscribe_request_for_device(checked_device, subscribe_request, capabilities_seen, attributes_seen, events_seen, subscribed_attributes, subscribed_events) | ||
| for _, component in pairs(checked_device.st_store.profile.components) do | ||
| function utils.populate_subscribe_request_for_device(checked_device, parent_device, subscribe_request, capabilities_seen, attributes_seen, events_seen, subscribed_attributes, subscribed_events) |
Contributor
There was a problem hiding this comment.
Could we just update this function to get the parent device of the checked_device as needed rather than passing on the parent device in the function parameters? If the device doesn't have a parent device, then we would know that it is the parent device.
ctowns
reviewed
Jun 3, 2026
| deviceTypes: | ||
| - id: 0x0145 # Snapshot Camera | ||
| deviceProfileName: camera | ||
| - id: "matter/chime" |
Contributor
There was a problem hiding this comment.
A device that supports Camera and Chime would equally match to the generic fingerprint for both Matter Camera and Matter Chime, so it is ambiguous which profile would be used initially. I know we would expect this to be fixed by the match profile logic, but could you add an additional fingerprint that matches on both the Camera and Chime device types and uses the camera profile? Also, this would allow us to set the default name to "Matter Camera" for these devices.
This adds support for the following device types: * 0x0140 Intercom * 0x0141 Audio Doorbell * 0x0142 Camera * 0x0143 Video Doorbell * 0x0144 Floodlight Camera * 0x0145 Snapshot Camera * 0x0146 Chime * 0x0148 DoorBell
nickolas-deboom
force-pushed
the
support-additional-camera-device-types
branch
from
0080716 to
394d80d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds support for the following device types: