SmartThingsEdgeDrivers icon indicating copy to clipboard operation
SmartThingsEdgeDrivers copied to clipboard

Matter-Sensor: Resolve issues with IKEA AQS

Open nickolas-deboom opened this issue 1 year ago • 5 comments

This pull request contains updates to the matter-sensor driver to resolve issues for the IKEA air quality sensor with the latest matter 1.2 driver.

nickolas-deboom avatar May 02 '24 16:05 nickolas-deboom

Duplicate profile check: Warning - duplicate profiles detected. air-quality-sensor-common.yml == air-quality-sensor-temp-humidity-co2-pm25-tvoc-meas.yml air-quality-sensor.yml == air-quality-sensor-AQI-only.yml

github-actions[bot] avatar May 02 '24 16:05 github-actions[bot]

Channel deleted.

github-actions[bot] avatar May 02 '24 16:05 github-actions[bot]

Test Results

   59 files   -  1    365 suites   - 5   0s :stopwatch: ±0s 1 726 tests  - 54  1 688 :white_check_mark:  - 54  0 :zzz: ±0  0 :x: ±0  38 :fire: ±0  3 015 runs   - 80  2 947 :white_check_mark:  - 80  0 :zzz: ±0  0 :x: ±0  68 :fire: ±0 

For more details on these errors, see this check.

Results for commit 0166fcf1. ± Comparison against base commit 902aff23.

This pull request removes 57 and adds 3 tests. Note that renamed tests count towards both.
Handle added lifecycle - no host user
Handle added lifecycle - only regular user
Handle detection frequency capability
Handle doConfigure lifecycle -- e1
Handle doConfigure lifecycle -- t1
Handle occupancy attr
Handle preference: autoReportType in infoChanged
Handle preference: group1Sensors in infoChanged
Handle preference: group1Time in infoChanged
Handle preference: group2Sensors in infoChanged
…
Report from cluster 0xFC03, command 0x05 should be handled as: threeAxis(1050, -3, 9)
Report from cluster 0xFC03, command 0x05 should be handled as: threeAxis(1123,-130,-24)
gasDetector report should be handled

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 02 '24 16:05 github-actions[bot]

File Coverage
All files 55% :x:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-sensor/src/PressureMeasurement/server/attributes/init.lua 94% :white_check_mark:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-sensor/src/PressureMeasurement/server/attributes/MeasuredValue.lua 90% :white_check_mark:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-sensor/src/PressureMeasurement/init.lua 71% :x:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-sensor/src/PressureMeasurement/types/Feature.lua 66% :x:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-sensor/src/PressureMeasurement/server/commands/init.lua 57% :x:
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-sensor/src/init.lua 34% :x:

Minimum allowed coverage is 90%

Generated by :monkey: cobertura-action against 0166fcf17af1a00d69735544a72cad459a918cc0

github-actions[bot] avatar May 02 '24 16:05 github-actions[bot]

Please note that these changes have not been tested yet and will need test case updates before closing.

nickolas-deboom avatar May 02 '24 18:05 nickolas-deboom

I want to get on the same page here, so bear with me.

So, I did a quick calculation and came up with 4194303 possible different profiles. So, are we planning on adding profiles as they are requested?

Yes, our plan is to add profiles to support certain combinations of functionality as they appear in the market. WWST is one way of us directly adding support for a specific device and creating a profile for it, but we can also utilize the log message that is output to see what profile would have been used if it were available, and then add the profile as needed.

Is it the App plugins or the profile system which disallows us from doing this dynamically?

Our static profile system is what is holding us back here. The dynamic devices project is currently in the works and that would be a better solution to this issue that would allow us to dynamically construct profiles for devices based on an onboarding interview. This is essentially trying to do that within the constraints of our static profiling system. So, we can dynamically construct profile names based on device functionality, and if we have a static profile that matches that name/set of functionality, then we can profile switch to that profile. However, if it does not exists, then we attempt to find a "best fit match" of one of our more common static profiles.

This is not a perfect system, but I think it provides some more flexibility given the constraints of the static profile architecture. In the future we should be able to do this dynamically.

ctowns avatar May 21 '24 19:05 ctowns

These changes were tested using an IKEA Vindstyrka Air Quality Sensor and Dirigera Hub, joined to a v3 hub via a matter bridge, and verifying that the Vindstyrka device joins to the correct profile for this device (air-quality-sensor-temp-humidity-tvoc-level-pm25-meas.yml). The airQualityHealthConcern, temperatureMeasurement, relativeHumidityMeasurement, fineDustSensor, and tvocHealthConcern capabilities all function as expected in the ST app.

nickolas-deboom avatar May 22 '24 17:05 nickolas-deboom

My understanding is that CHAD-13070 is for integrating the Ikea Air Quality Sensor for Matter 1.2. Could you help me understand why multiple profiles are added as part of this PR? Perhaps I am misunderstanding the scope.

lelandblue avatar May 22 '24 21:05 lelandblue

My understanding is that CHAD-13070 is for integrating the Ikea Air Quality Sensor for Matter 1.2. Could you help me understand why multiple profiles are added as part of this PR? Perhaps I am misunderstanding the scope.

The goal of this PR is to add more robust profile switching for AQS devices. The title of this PR should be changed to account for that. The initial reason this PR was made was to dynamic profile selection for the IKEA AQS, but ultimately we generalized this solution so that it could work for multiple devices, including ones we don't know about yet.

So yes, this will add support for the IKEA AQS, but it will also improve our support for AQS devices in general through more robust profile switching. The other profiles that are added as a part of this PR represent either duplicates of existing profiles that follow our new naming scheme (and the old profiles will be deleted) or new profiles that use our new profile building scheme and cover some basic common capability combinations.

ctowns avatar May 23 '24 01:05 ctowns