[WWSTCERT-13550] Sonoff - Add the pressure display function to SNZB-02M - #3194
[WWSTCERT-13550] Sonoff - Add the pressure display function to SNZB-02M#3194laity-w-sudo wants to merge 12 commits into
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Profile category check: ✅ Passed - all profiles have a category defined. |
|
Invitation URL: |
Test Results 73 files ±0 542 suites +1 0s ⏱️ ±0s For more details on these errors, see this check. Results for commit 1cd32e1. ± Comparison against base commit 71bbd3d. ♻️ This comment has been updated with latest results. |
|
zigbee-humidity-sensor_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against 1cd32e1 |
The SNZB-02M sub-driver was added but never registered, so the pressure display feature was dead code on the device. Register it via sub_drivers.lua, drop the unrelated SNZB-02LD/WD handler and utils (not in this driver, and referencing a nonexistent sub-driver), and add unit tests for pressure conversion and refresh. Co-Authored-By: Claude <noreply@anthropic.com>
ae161d5 to
b5e3665
Compare
cjswedes
left a comment
There was a problem hiding this comment.
My biggest concern is the lack of configuration for reporting the PressureMeasurement MeasuredValue attribute; has this been tested manually?
There was a problem hiding this comment.
Is there a reason to have the nested directories? We should just add a sonoff subdriver rather than having sonoff/SNZB-02M. We want to avoid having single subdrivers for each model.
There was a problem hiding this comment.
Please change the directory structure so that it is not nested, and everything falls under the sonoff directory.
| zigbee_handlers = { | ||
| attr = { | ||
| [clusters.PressureMeasurement.ID] = { | ||
| [clusters.PressureMeasurement.attributes.MeasuredValue.ID] = pressure_report_handler |
There was a problem hiding this comment.
Did you test that the PressureMeasurement attribute is reported automatically when the value changes? I do not see any reporting configuration for the attribute or binding of the cluster. I would expect to see a call in a device_init handler to device:add_configured_attribute to ensure this configuration is setup when the device is configured.
There was a problem hiding this comment.
Most clusters get configured due to default functionality, but I dont believe we are configuring this one with the defaults, so the subdriver should be doing so.
There was a problem hiding this comment.
@laity-w-sudo
here is the example of cluster configuration in the subdriver:
Check all that apply
Type of Change
Checklist
Description of Change
Adding the air pressure function to the SNZB-02M equipment
Summary of Completed Tests