Skip to content

Matter Camera: Exclude audioVolume on unusable volume range - #3242

Open
samadDotDev wants to merge 4 commits into
mainfrom
matter-usuable-volume-range
Open

samadDotDev wants to merge 4 commits into
mainfrom
matter-usuable-volume-range

Conversation

@samadDotDev

Copy link
Copy Markdown
Contributor

Description of Change

In Matter spec, there’s no specific feature-map based gating on microphone/speaker volume settings, outside of the larger Audio (ADO) feature i.e. any camera device supporting audio needs to implement these settings even if the hardware doesn’t support adjusting volume/gain for it. We have encountered one such camera and have confirmed with the partner that this setting cannot be implemented for their hardware.

Upon following up with Camera TT in CSA, it was noted as a miss that will be followed up separately (to either change the conformance of volume settings from ADO to [ADO], or introduce a separate feature map bit for audio gain control) in the future versions of the spec. However to resolve issues in the field and currently available versions of the spec, we found a backward-compatible workaround (which was confirmed to be allowed per DM TT) where min and max can be set to the same value in the constraints defined for these attributes, and that can be keyed off by clients to determine that the volume range is unusable and therefore the capability can be excluded.

Summary of Completed Tests

  • Tested with Matter cameras including usable ranges
  • Added unit tests exercising both cases (where these capabilities should be included or excluded)
  • [Pending test once the beta firmware is released] To-test with aforementioned Matter camera that will include an unusable range.

@github-actions

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

matter-switch_coverage.xml

File Coverage
All files 93%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/eve_energy/init.lua 91%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/device_configuration.lua 97%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/camera/camera_utils/utils.lua 99%
/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/aqara_cube/init.lua 95%
/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/sub_drivers/camera/camera_handlers/attribute_handlers.lua 96%
/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/ikea_scroll/scroll_utils/event_utils.lua 52%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-switch/src/sub_drivers/hager/init.lua 95%
/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/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/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/init.lua 90%
/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/init.lua 97%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against b6314ae

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Test Results

   73 files    541 suites   0s ⏱️
3 276 tests 3 276 ✅ 0 💤 0 ❌
5 265 runs  5 265 ✅ 0 💤 0 ❌

Results for commit b6314ae.

♻️ This comment has been updated with latest results.

@ldeora

ldeora commented Sep 21, 2026

Copy link
Copy Markdown

The equal min/max convention seems like a reasonable compatibility mechanism for devices whose hardware cannot provide adjustable gain/volume, and handling speaker and microphone independently makes sense.

I noticed two cases that I think should be addressed.

1. Range discovery currently depends on audioVolume already being present

should_include_volume_capability() deliberately preserves the existing capability state while either raw bound is unknown:

if raw_min_volume == nil or raw_max_volume == nil then
  return device:supports_capability(capabilities.audioVolume, component)
end

The problem is that the normal camera subscription currently obtains SpeakerMinLevel / SpeakerMaxLevel and the microphone equivalents through the audioVolume capability mapping itself.

That creates a bootstrap dependency:

audioVolume not currently enabled
    ↓
min/max are not subscribed
    ↓
raw range remains unknown
    ↓
should_include_volume_capability() returns false
    ↓
audioVolume cannot be added

The test setup appears to demonstrate this as well: seed_usable_volume_range() has to populate the raw ranges before update_device_profile() so that the normal camera profile still gets audioVolume.

I think range discovery should therefore be independent of whether audioVolume is currently exposed - for example, an initial READ/subscription of the relevant min/max attributes, ideally gated by the AVSM metadata/AttributeList. The discovered range can then decide whether the public capability belongs in the profile.

This also matters after audioVolume has been removed: if the raw fields are lost on restart, the driver otherwise has no path to discover that a later firmware/device state provides a usable range again.

2. The existing range validation still treats the new equal-range convention as invalid

The current handlers use:

max_volume <= min_volume
min_volume >= max_volume

as invalid-range conditions and replace the reported value with the fallback bound.

With the convention introduced by this PR, min == max is intentional and should no longer go through the malformed-range path. The raw fields preserve the original values, so the profile decision still works, but the driver's validated internal range then disagrees with the new semantics and an intentional constant range produces an "invalid" warning.

The released Matter 1.5 camera model requires these range attributes with the corresponding SPKR/ADO features, and the Matter 1.5 AVSM conformance test explicitly checks:

SpeakerMinLevel <= SpeakerMaxLevel
MicrophoneMinLevel <= MicrophoneMaxLevel

so equality is not an invalid range.

I think the driver should distinguish:

min < max  → usable adjustable range
min == max → valid but non-adjustable range; omit audioVolume
min > max  → malformed range

and avoid attempting normal volume normalization when the known range is non-adjustable.

A regression test starting with no audioVolume and no cached raw range, then discovering a usable range and adding the capability, would cover the first issue nicely.

Other than these two points, the overall approach looks good.

@samadDotDev

Copy link
Copy Markdown
Contributor Author

Good catch on both.

For # 1: you're right, it wasn't actually decoupled. SpeakerMaxLevel/MinLevel and the microphone equivalents were only subscribed as part of audioVolume's own attribute group, so a device without audioVolume yet would never receive the values needed to grant it. Fixed by moving those four discovery attributes into the audioMute group instead, which is added unconditionally whenever the AUDIO/SPEAKER feature is present. Therefore, the range discovery no longer depends on audioVolume already being active, and it also survives audioVolume being dropped and re-evaluated later. Added a regression test that starts from an unseeded device and asserts the subscribe request carries the min/max attributes before audioVolume is granted.

For # 2: the capability-inclusion check already treats min == max and min > max the same (both omit audioVolume), so no change needed there. I did tighten the separate logging path in the min/max handlers, which was still using <=/>= and would log a false "invalid range" warning + clamp an intentional constant range. This is now changed to strict </> per the AVSM conformance check you linked.

@ldeora

ldeora commented Sep 21, 2026

Copy link
Copy Markdown

Thanks! Both issues I raised are now addressed. Moving the range attributes into the audioMute subscription group resolves the discovery dependency, and the strict comparisons correctly accept min == max.

One small remaining edge case: volume_level_handler() still divides by max_volume - min_volume. A guard for a known zero-width range would prevent a late volume report from triggering division by zero while the profile is being updated.

Other than that, the changes look good to me.

@samadDotDev

Copy link
Copy Markdown
Contributor Author

@Ideora Good catch again, confirmed with a test. Once min == max is accepted as valid, volume_level_handler()'s (value - min) * 100 / (max - min) divides by zero for any report that arrives before the narrowed profile (dropping audioVolume) round-trips back down, and the resulting nan gets rejected by the capability layer, crashing the driver.

Added a guard: when the range is zero-width, it reports a fixed 0 instead of normalizing. Also checked the volume math in capability_handlers.lua (set/up/down) - volume_range is only ever used as a multiplier there, not a divisor, so it isn't exposed to the same failure, and those commands can't reach the driver once audioVolume is hidden anyway. Added a regression test that reproduces the race (narrows the range, then injects a late volume report) and confirmed it crashes without the guard and passes with it.

@Kwang-Hui Kwang-Hui left a comment

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.

Looks good to me

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.

3 participants