Skip to content

Fix non-END 0x16 leading to early frame termination #3

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yzhang318
Copy link

@yzhang318 yzhang318 commented Nov 7, 2023

Hi,

I'm submitting this PR to solve a bug that leads to occasional checksum failures.

Issue Description

The 2nd-last byte of a frame is CS (Checksum), and the last byte (END) is a fixed 0x16. When CS happens to be 0x16, it is taken as END to terminate the frame. Later checksum verification fails since the framedata[-2] is mistaken as CS.

Another potential problem could be that there is 0x16 in addr section. However, it could be circumvented by using the broadcast address.

Solution

A state machine is written to recognize the 0xFE -> 0x68 -> addr -> 0x68 -> ctl_code -> data -> CS -> 0x16 format.

8 states are provided:

  1. start, looking for 0xFE
  2. [optional] awaken received, looking for 1st 0x68 (b_start)
  3. 1st 0x68 received, looking for 2nd 0x68
  4. 2nd 0x68 byte received, next byte is ctl_code
  5. ctl_code received, next byte is length
  6. length byte received, set counter to find CS
  7. CS byte received, waiting for b_end
  8. b_end received

Verification

The fixed code is applied to read from a real meter. Before this fix, occasional frame checksum errors can be observed. After implementing the proposed modification, no more checksum errors are encountered.


Thank you for your great effort to provide this library! ☕️

@yzhang318 yzhang318 changed the title Fix non-checksum 0x16 leading to early frame termination Fix non-END 0x16 leading to early frame termination Nov 7, 2023
@hrbonz
Copy link
Contributor

hrbonz commented Mar 11, 2024

Only just saw this PR, my apologies for the delay. Could you merge both your commits as the second one seems to rewrite the first one anyway.
The biggest effort was to read the standard in Chinese!

@yzhang318
Copy link
Author

@hrbonz Hi! It's great to be able to help! I've reviewed the branch and am merging the two commits you mentioned. I totally get it - reading the documentation takes a lot of effort. Thanks so much for all your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants