lib: zcl: handle the float types as analog data - #146
Open
EdouardMALOT wants to merge 2 commits into
Open
EdouardMALOT wants to merge 2 commits into
EdouardMALOT wants to merge 2 commits into
Conversation
The ZCL spec lists the semi, single and double precision floats among the analog data types, so a Configure Reporting record for such an attribute carries a reportable change field of the type's size. zb_zcl_is_analog_data_type() stopped at the integers, and zb_zcl_get_analog_attribute_size() and zb_zcl_fix_endian() had no case for them either: a client configuring reporting on a single float attribute sent the record without the field and the device answered MALFORMED_COMMAND, and the Read Reporting Configuration Response it parsed was cut after the intervals. zb_zcl_put_value_to_packet() knew single alone. Seen from an nRF52840 coordinator with a CO2 sensor, cluster 0x040D attribute 0x0000, type 0x39. Signed-off-by: EdouardMalot <edouard.malot@gmail.com>
With the float types now analog, the server side of reporting fell into the default branch of its switches: no reported value saved, no delta compared, and a Read Reporting Configuration Response without its reportable change. Compare single and double the way the integers are. The double lands in the union as raw bytes so its alignment does not change the layout of zb_zcl_reporting_info_t. The delta of a semi float is emitted in the response but not compared: the targets have no half precision type, so such an attribute keeps reporting every change, as the 64-bit integers do today. Signed-off-by: EdouardMalot <edouard.malot@gmail.com>
Collaborator
|
Hi @EdouardMALOT, thanks for the contribution. The ZCL code is released as open source, however, it belongs to the ZBOSS stack tree and the fix needs to be addressed through the ZOI project. We can't merge this PR, but I've already reported the issue internally. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The ZCL floating point types (semi 0x38, single 0x39, double 0x3A) are analog data types, but
lib/zboss/src/zcl/zcl_common.cdoes not know them:zb_zcl_is_analog_data_type()answers false,zb_zcl_get_analog_attribute_size()returns 0 andzb_zcl_fix_endian()has no case, whilezb_zcl_put_value_to_packet()knows single alone.Observed from an nRF52840 coordinator on v1.4.0, configuring reporting on a CO2 sensor (cluster 0x040D, attribute 0x0000, type 0x39):
First commit adds the three types to the four switches of
zcl_common.c.Second commit keeps the server side coherent, since
zcl_reporting.candzb_zcl_read_report_config_cmd_handler()now reach their analog branch for these types: single and double are saved and compared like the integers (the double is stored as raw bytes so the union keeps its 4-byte alignment andzb_zcl_reporting_info_tits layout); semi is emitted in the Read Reporting Configuration Response but not compared, there is no half precision type on the targets, so it keeps the "report every change" behaviour the 64-bit integers have today.Testing so far: an nRF52840 coordinator application built against this branch, with the application-side workaround (
--wrapofzb_zcl_is_analog_data_type()) removed. The single case is being run on a bench with the CO2 sensor above; semi and double are covered by reading and by the build only, no device at hand.