Skip to content
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

Space for improvement at uart_iqrf_read #7

Open
HSyr opened this issue May 23, 2019 · 0 comments
Open

Space for improvement at uart_iqrf_read #7

HSyr opened this issue May 23, 2019 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@HSyr
Copy link

HSyr commented May 23, 2019

  1. When receiverControl.wasEscape == 1, then the code accepts both HDLC_FRM_FLAG_SEQUENCE and HDLC_FRM_CONTROL_ESCAPE characters, although this should not happen. There is a high probability (99.6%) that such a situation will be detected later by invalid CRC, but it is not 100% sure.

  2. The function waits at the beginning for the starting HDLC_FRM_FLAG_SEQUENCE character (so it is not expected it is the 1st character received). That implies that the reception of the HDLC packet might start in the middle of another one. Then if the algorithm reads ending HDLC_FRM_FLAG_SEQUENCE of such a packet it understands it as a starting HDLC_FRM_FLAG_SEQUENCE of the new packet, which is wrong as it will get out of sync. I think the function should always try to receive the correct HDLC packet within the specified timeout interval to get in-sync with HDLC protocol.

  3. Also the magic literal constants used in the code would deserve at least some comment to explain them.

SLEEP(5);
...
timeout += 500;
@spinarr spinarr added the enhancement New feature or request label Aug 14, 2020
@spinarr spinarr assigned spinarr and khgreav and unassigned spinarr Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants