-
Notifications
You must be signed in to change notification settings - Fork 24
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.
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
GH-20: Fix support for COMMAND_CLASS_METER V1,V2 & V3 #20
GH-20: Fix support for COMMAND_CLASS_METER V1,V2 & V3 #20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some nitpicky feedback
@@ -205,13 +205,21 @@ static sl_status_t zwave_command_class_meter_get(attribute_store_node_t node, | |||
attribute_store_node_t scale_node | |||
= attribute_store_get_first_parent_with_type(rate_type_node, | |||
ATTRIBUTE(SCALE)); | |||
zwave_cc_version_t supporting_node_version | |||
= zwave_command_class_get_version_from_node(node, COMMAND_CLASS_METER); | |||
|
|||
meter_scale_t scale = 0; | |||
attribute_store_get_reported(scale_node, &scale, sizeof(scale)); | |||
|
|||
ZW_METER_GET_V5_FRAME *get_frame = (ZW_METER_GET_V5_FRAME *)frame; | |||
get_frame->cmdClass = COMMAND_CLASS_METER_V6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related to you pr, but I would expect to have v6 frame to align the cmd class version maybe different ones will work but I bring confusion IMHO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
V6 is only locally defined
UnifySDK/applications/zpc/components/zwave_command_classes/src/zwave_command_class_meter_control.c
Line 42 in db3a8e6
// ZW_classcmd does not know about v6. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok this should have been done differently, I'll see how to avoid this kind of duplication...
FTR at zwave we want to generate headers from xml file, unify should do same...
applications/zpc/components/zwave_command_classes/src/zwave_command_class_meter_control.c
Outdated
Show resolved
Hide resolved
applications/zpc/components/zwave_command_classes/src/zwave_command_class_meter_control.c
Outdated
Show resolved
Hide resolved
e981dce
to
db3a8e6
Compare
Fixes bug where Attribute [0x3208] Value can not get resolved for legacy devices. Size of ZW_METER_GET_V3_FRAME is smaller than size of ZW_METER_GET_V5_FRAME. Legacy device supporting COMMAND_CLASS_METER_V3 and below will ignore received V5 frames, because frame size is larger than expected. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record related links:
Build: https://github.com/nkljajic/UnifySDK/actions/runs/7019839630/job/19098211543 : success
Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-20/develop
…s of CC Bug-SiliconLabs: UIC-3082 Origin: https://github.com/rzr/UnifySDK/tree/GH-20/UIC-3082/phcoval/main Relate-to: nkljajic#1 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-20/develop Signed-off-by: Philippe Coval <philippe.coval@silabs.com>
Blocked-by: nkljajic#1 This change should comes along test fixes as verified at: https://github.com/rzr/UnifySDK/actions/runs/7544676005/job/20538549227 |
SiliconLabsGH-14: Enable testing by default
May your rebase on main to avoid the merge commit I confident it will be part of upcoming unify release Thank you for your contribution |
Fixes bug where Attribute [0x3208] Value can not get resolved for legacy devices. Size of ZW_METER_GET_V3_FRAME is smaller than size of ZW_METER_GET_V5_FRAME. Legacy device supporting COMMAND_CLASS_METER_V3 and below will ignore received V5 frames, because frame size is larger than expected. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>
More to be tested Bug-SiliconLabs: UIC-3082 Bug-GitHub: rzr#3 Origin: https://github.com/rzr/UnifySDK/tree/GH-14/UIC-3082/phcoval/main Forwarded: SiliconLabs#26 Signed-off-by: Philippe Coval <philippe.coval@silabs.com> (cherry picked from commit 92f99dc) Signed-off-by: Philippe Coval <philippe.coval@silabs.com>
…s of CC Bug-SiliconLabs: UIC-3082 Origin: https://github.com/rzr/UnifySDK/tree/GH-20/UIC-3082/phcoval/main Relate-to: #1 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-20/develop Signed-off-by: Philippe Coval <philippe.coval@silabs.com>
653aa87
to
9c4b669
Compare
Origin-SiliconLabs: ver_1.5.0-unstable-79-g24f0d9eb1 |
Fixes bug where Attribute [0x3208] Value can not get resolved for legacy devices. Size of ZW_METER_GET_V3_FRAME is smaller than size of ZW_METER_GET_V5_FRAME. Legacy device supporting COMMAND_CLASS_METER_V3 and below will ignore received V5 frames, because frame size is larger than expected. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>
Change
Fixes bug where Attribute [0x3208] Value can not get resolved for legacy devices.
Size of ZW_METER_GET_V3_FRAME is smaller than size of ZW_METER_GET_V5_FRAME.
Legacy device supporting COMMAND_CLASS_METER_V3 and below will ignore received V5 frames,
because frame size is larger than expected.
Checklist