Skip to content

Matter Camera: Exclude audioVolume on unusable volume range #3242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
samadDotDev merged 5 commits into from
Sep 28, 2026
Merged

Matter Camera: Exclude audioVolume on unusable volume range #3242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9866fc1
CHAD-19242: Exclude audioVolume for unusable range
samadDotDev Sep 21, 2026
3ffcbca
Keep the capability as-is when ranges are unknown
samadDotDev Sep 21, 2026
0750c21
Update gating for min/max attr subscription, fix log
samadDotDev Sep 21, 2026
b6314ae
Guard against divide-by-zero
samadDotDev Sep 22, 2026
d29da26
Track the raw value directly
samadDotDev Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions ...s/SmartThings/matter-switch/src/sub_drivers/camera/camera_handlers/attribute_handlers.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,34 +57,48 @@ end

function CameraAttributeHandlers.volume_level_handler(driver, device, ib, response)
local component = device:endpoint_to_component(ib)
local max_volume = device:get_field(camera_fields.MAX_VOLUME_LEVEL .. "_" .. component) or camera_fields.ABS_VOL_MAX
local min_volume = device:get_field(camera_fields.MIN_VOLUME_LEVEL .. "_" .. component) or camera_fields.ABS_VOL_MIN
-- Convert from [min_volume, max_volume] to [0, 100] before emitting capability
local max_volume = camera_utils.get_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, component) or camera_fields.ABS_VOL_MAX
local min_volume = camera_utils.get_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, component) or camera_fields.ABS_VOL_MIN
local limited_range = max_volume - min_volume
if limited_range <= 0 then
device:emit_event_for_endpoint(ib, capabilities.audioVolume.volume(0))
return
end
-- Convert from [min_volume, max_volume] to [0, 100] before emitting capability
local normalized_volume = utils.round((ib.data.value - min_volume) * 100.0 / limited_range)
device:emit_event_for_endpoint(ib, capabilities.audioVolume.volume(normalized_volume))
end

function CameraAttributeHandlers.max_volume_level_handler(driver, device, ib, response)
local component = device:endpoint_to_component(ib)
local max_volume = ib.data.value
local min_volume = device:get_field(camera_fields.MIN_VOLUME_LEVEL .. "_" .. component)
if max_volume > camera_fields.ABS_VOL_MAX or (min_volume and max_volume <= min_volume) then
local min_volume = camera_utils.get_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, component)
-- min == max is a valid (non-adjustable) range per the Matter spec, so only max < min is malformed;
-- an out-of-bounds value is clamped, but a malformed relationship is only logged and left as-is,
-- since volume_level_handler already guards against a non-positive range.
Comment on lines +76 to +78

@hcarter-775 hcarter-775 Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove comment? Seems unnecessary to mention this

@ctowns ctowns Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think the comment is useful since it documents atypical behavior (min == max being allowed)

@hcarter-775 hcarter-775 Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that part is fine. Mostly I was mentioning the extra stuff about only logging the malformed relationship, which is explicitly written in code just below. No strong feelings, but that is what I was primarily pointing out

if max_volume > camera_fields.ABS_VOL_MAX then
device.log.warn(string.format("Device reported invalid maximum (%d) %s volume level range value", ib.data.value, component))
max_volume = camera_fields.ABS_VOL_MAX
elseif min_volume and max_volume < min_volume then
device.log.warn(string.format("Device reported invalid maximum (%d) %s volume level range value", ib.data.value, component))
end
device:set_field(camera_fields.MAX_VOLUME_LEVEL .. "_" .. component, max_volume)
camera_utils.set_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, component, max_volume)
camera_cfg.reconcile_profile_and_capabilities(device)
end

function CameraAttributeHandlers.min_volume_level_handler(driver, device, ib, response)
local component = device:endpoint_to_component(ib)
local min_volume = ib.data.value
local max_volume = device:get_field(camera_fields.MAX_VOLUME_LEVEL .. "_" .. component)
if min_volume < camera_fields.ABS_VOL_MIN or (max_volume and min_volume >= max_volume) then
local max_volume = camera_utils.get_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, component)
-- See max_volume_level_handler: min == max is valid, only min > max is malformed and only logged.

@hcarter-775 hcarter-775 Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same thing here about unnecessary comment

if min_volume < camera_fields.ABS_VOL_MIN then
device.log.warn(string.format("Device reported invalid minimum (%d) %s volume level range value", ib.data.value, component))
min_volume = camera_fields.ABS_VOL_MIN
elseif max_volume and min_volume > max_volume then
device.log.warn(string.format("Device reported invalid minimum (%d) %s volume level range value", ib.data.value, component))
end
device:set_field(camera_fields.MIN_VOLUME_LEVEL .. "_" .. component, min_volume)
camera_utils.set_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, component, min_volume)
Comment thread
hcarter-775 marked this conversation as resolved.
camera_cfg.reconcile_profile_and_capabilities(device)
end

function CameraAttributeHandlers.status_light_enabled_handler(driver, device, ib, response)
Expand Down
12 changes: 6 additions & 6 deletions .../SmartThings/matter-switch/src/sub_drivers/camera/camera_handlers/capability_handlers.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ end

function CameraCapabilityHandlers.handle_set_volume(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
local max_volume = device:get_field(camera_fields.MAX_VOLUME_LEVEL .. "_" .. cmd.component) or camera_fields.ABS_VOL_MAX
local min_volume = device:get_field(camera_fields.MIN_VOLUME_LEVEL .. "_" .. cmd.component) or camera_fields.ABS_VOL_MIN
local max_volume = camera_utils.get_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, cmd.component) or camera_fields.ABS_VOL_MAX
local min_volume = camera_utils.get_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, cmd.component) or camera_fields.ABS_VOL_MIN
-- Convert from [0, 100] to [min_volume, max_volume] before writing attribute
local volume_range = max_volume - min_volume
local volume = utils.round(cmd.args.volume * volume_range / 100.0 + min_volume)
Expand All @@ -75,8 +75,8 @@ end

function CameraCapabilityHandlers.handle_volume_up(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
local max_volume = device:get_field(camera_fields.MAX_VOLUME_LEVEL .. "_" .. cmd.component) or camera_fields.ABS_VOL_MAX
local min_volume = device:get_field(camera_fields.MIN_VOLUME_LEVEL .. "_" .. cmd.component) or camera_fields.ABS_VOL_MIN
local max_volume = camera_utils.get_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, cmd.component) or camera_fields.ABS_VOL_MAX
local min_volume = camera_utils.get_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, cmd.component) or camera_fields.ABS_VOL_MIN
local volume = device:get_latest_state(cmd.component, capabilities.audioVolume.ID, capabilities.audioVolume.volume.NAME)
if not volume or volume >= max_volume then return end
-- Convert from [0, 100] to [min_volume, max_volume] before writing attribute
Expand All @@ -91,8 +91,8 @@ end

function CameraCapabilityHandlers.handle_volume_down(driver, device, cmd)
local endpoint_id = device:component_to_endpoint(cmd.component)
local max_volume = device:get_field(camera_fields.MAX_VOLUME_LEVEL .. "_" .. cmd.component) or camera_fields.ABS_VOL_MAX
local min_volume = device:get_field(camera_fields.MIN_VOLUME_LEVEL .. "_" .. cmd.component) or camera_fields.ABS_VOL_MIN
local max_volume = camera_utils.get_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, cmd.component) or camera_fields.ABS_VOL_MAX
local min_volume = camera_utils.get_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, cmd.component) or camera_fields.ABS_VOL_MIN
local volume = device:get_latest_state(cmd.component, capabilities.audioVolume.ID, capabilities.audioVolume.volume.NAME)
if not volume or volume <= min_volume then return end
-- Convert from [0, 100] to [min_volume, max_volume] before writing attribute
Expand Down
21 changes: 19 additions & 2 deletions ...rs/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/device_configuration.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ local function set_hard_privacy_mode_presence(device, hard_privacy_mode_present)
device:set_field(camera_fields.HARD_PRIVACY_MODE_PRESENT, hard_privacy_mode_present == true, { persist = true })
end

--- Leaves audioVolume as-is if the min/max levels aren't both known yet instead of guessing;
--- otherwise requires max > min to include it.
--- @param component string a profile component name
--- @return should_include boolean whether audioVolume belongs on the component argument.
local function should_include_volume_capability(device, component)
local min_volume = camera_utils.get_field_for_component(device, camera_fields.MIN_VOLUME_LEVEL, component)
local max_volume = camera_utils.get_field_for_component(device, camera_fields.MAX_VOLUME_LEVEL, component)
if min_volume == nil or max_volume == nil then
return device:supports_capability(capabilities.audioVolume, component)
end
return max_volume > min_volume
end

local function build_webrtc_supported_features()
return {
bundle = true,
Expand Down Expand Up @@ -258,7 +271,9 @@ function CameraDeviceConfiguration.match_profile(device)
table.insert(main_component_capabilities, capabilities.audioRecording.ID)
end
table.insert(microphone_component_capabilities, capabilities.audioMute.ID)
table.insert(microphone_component_capabilities, capabilities.audioVolume.ID)
if should_include_volume_capability(device, camera_fields.profile_components.microphone) then
table.insert(microphone_component_capabilities, capabilities.audioVolume.ID)
end
end
if clus_has_feature(clusters.CameraAvStreamManagement.types.Feature.SNAPSHOT) then
table.insert(main_component_capabilities, capabilities.imageCapture.ID)
Expand All @@ -268,7 +283,9 @@ function CameraDeviceConfiguration.match_profile(device)
end
if clus_has_feature(clusters.CameraAvStreamManagement.types.Feature.SPEAKER) then
table.insert(speaker_component_capabilities, capabilities.audioMute.ID)
table.insert(speaker_component_capabilities, capabilities.audioVolume.ID)
if should_include_volume_capability(device, camera_fields.profile_components.speaker) then
table.insert(speaker_component_capabilities, capabilities.audioVolume.ID)
end
end
if clus_has_feature(clusters.CameraAvStreamManagement.types.Feature.IMAGE_CONTROL) then
table.insert(main_component_capabilities, capabilities.imageControl.ID)
Expand Down
22 changes: 17 additions & 5 deletions drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ function CameraUtils.feature_supported(device, cluster_id, feature_flag)
return #device:get_endpoints(cluster_id, { feature_bitmap = feature_flag }) > 0
end

function CameraUtils.get_field_for_component(device, field, component)
return device:get_field(string.format("%s_%s", field, component))
end

function CameraUtils.set_field_for_component(device, field, component, value, additional_params)
device:set_field(string.format("%s_%s", field, component), value, additional_params)
end

function CameraUtils.update_supported_attributes(device, ib, capability, attribute)
local attribute_set = device:get_latest_state(
camera_fields.profile_components.main, capability.ID, capability.supportedAttributes.NAME
Expand Down Expand Up @@ -212,18 +220,22 @@ function CameraUtils.subscribe(device)
clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMax,
clusters.CameraAvSettingsUserLevelManagement.attributes.TiltMin
},
-- SpeakerMaxLevel/MinLevel and MicrophoneMaxLevel/MinLevel are subscribed alongside audioMute
-- (unconditional on the AUDIO/SPEAKER feature) rather than audioVolume, since audioVolume's own
-- presence in the profile depends on first discovering these values -- gating their subscription
-- on audioVolume already being present would make that discovery impossible.
Comment on lines +223 to +226

@hcarter-775 hcarter-775 Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a bit of work (kinda outdated but not too much I don't think) trying to solve this exact problem a little more clearly (that is, adding the ability to subscribe to attributes/events based on other values besides a capability.

@samadDotDev samadDotDev Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks neat! We can move to those conditional_subscriptions post this change (and once you rebase your work later).

[capabilities.audioMute.ID] = {
clusters.CameraAvStreamManagement.attributes.SpeakerMuted,
clusters.CameraAvStreamManagement.attributes.MicrophoneMuted
},
[capabilities.audioVolume.ID] = {
clusters.CameraAvStreamManagement.attributes.SpeakerVolumeLevel,
clusters.CameraAvStreamManagement.attributes.MicrophoneMuted,
clusters.CameraAvStreamManagement.attributes.SpeakerMaxLevel,
clusters.CameraAvStreamManagement.attributes.SpeakerMinLevel,
clusters.CameraAvStreamManagement.attributes.MicrophoneVolumeLevel,
clusters.CameraAvStreamManagement.attributes.MicrophoneMaxLevel,
clusters.CameraAvStreamManagement.attributes.MicrophoneMinLevel
},
[capabilities.audioVolume.ID] = {
clusters.CameraAvStreamManagement.attributes.SpeakerVolumeLevel,
clusters.CameraAvStreamManagement.attributes.MicrophoneVolumeLevel
},
[capabilities.mode.ID] = {
clusters.CameraAvStreamManagement.attributes.StatusLightBrightness
},
Expand Down
Loading
Loading