[ld2420] Fix sizeof vs value bug in register memcpy#14286
[ld2420] Fix sizeof vs value bug in register memcpy#14286swoboda1337 merged 1 commit intoesphome:devfrom
Conversation
sizeof(CMD_REG_DATA_REPLY_SIZE) evaluates to sizeof(uint8_t) = 1, but the intended copy size is the value of CMD_REG_DATA_REPLY_SIZE which is 2. This caused register read/write commands to only copy 1 byte instead of 2, corrupting the data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file: external_components:
- source: github://pr#14286
components: [ld2420]
refresh: 1h(Added by the PR bot) |
|
👋 Hi there! This PR modifies 1 file(s) with codeowners. @descipher - As codeowner(s) of the affected files, your review would be appreciated! 🙏 Note: Automatic review request may have failed, but you're still welcome to review. |
| ((buffer[CMD_FRAME_DATA_LENGTH] - 4) / CMD_REG_DATA_REPLY_SIZE)); | ||
| index += CMD_REG_DATA_REPLY_SIZE) { | ||
| memcpy(&this->cmd_reply_.data[reg_element], &buffer[data_pos + index], sizeof(CMD_REG_DATA_REPLY_SIZE)); | ||
| memcpy(&this->cmd_reply_.data[reg_element], &buffer[data_pos + index], CMD_REG_DATA_REPLY_SIZE); |
There was a problem hiding this comment.
Confirmed, it looks wrong, and it is wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #14286 +/- ##
=======================================
Coverage 74.28% 74.28%
=======================================
Files 55 55
Lines 11595 11595
Branches 1583 1583
=======================================
Hits 8613 8613
Misses 2578 2578
Partials 404 404 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes an LD2420 register read/write framing bug where sizeof(CMD_REG_DATA_REPLY_SIZE) (1 byte) was mistakenly used instead of the constant value (2 bytes), causing truncated memcpy operations during register access.
Changes:
- Correct memcpy lengths in
handle_ack_data_()when parsing register read reply data. - Correct memcpy lengths in
set_reg_value()when serializing the register address and value.
|
Thanks |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
What does this implement/fix?
Fixes a bug where
sizeof(CMD_REG_DATA_REPLY_SIZE)was used instead ofCMD_REG_DATA_REPLY_SIZEas the memcpy length at three call sites.CMD_REG_DATA_REPLY_SIZEis astatic constexpr uint8_t = 0x02, sosizeof()returns 1 (size of uint8_t) instead of the intended value of 2. This causes memcpy to copy only 1 byte instead of 2 for register read/write operations.Affected sites:
handle_ack_data_()line 593: reading register reply dataset_reg_value()lines 732, 734: writing register address and valueTypes of changes
Related issue or feature (if applicable):
N/A
Pull request in esphome-docs with documentation (if applicable):
N/A
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).