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

Add RC-specific 0xEE sync byte #30

Merged
merged 2 commits into from
Jan 11, 2025
Merged

Conversation

akumaigorodski
Copy link
Contributor

@akumaigorodski akumaigorodski commented Jan 7, 2025

Radiomaster controllers (TX12, TX16 models at least with EdgeTX v2.8.0-2.9.1) use 0xEE as RcChannelsPacked sync byte, but 0xCE for other types of messages, this makes parser to search for either of these two bytes to successfully parse all the messages.

RC messages happen much more often than other types of messages, this will speed up `contains` method on slice.
@tact1m4n3
Copy link
Owner

Hi! Thanks for your PR. However I am not sure that changing the Default implementation of the config struct is worth it in this case. To achieve the same effect, you can pass a custom config struct to the parser. On the other hand, I think that silently skipping bytes when we encounter an unrecognized sync byte (where we expect a packet to start) might not be the best way to tackle the situation. So as an alternative, I suggest we make a new parser error for this (InvalidSyncByte) and for the times we actually want to sync (search for the beginning of a packet) we can make a new method. This would at least give a heads up to the programmer that something is wrong.

@akumaigorodski
Copy link
Contributor Author

Well, I did it this exact way because there's specific instruction about it here: https://github.com/crsf-wg/crsf/wiki/Message-Format#sync

But I can do it as an error, too. In that case I would probably want that error to contain raw packet bytes, because what I work on is a proxy program which takes CRSF stream from RF module's UART, looks for interesting bits, and then sends it to flight controller UART anyway, so such RC data can't be discarded.

@tact1m4n3
Copy link
Owner

Well, I did it this exact way because there's specific instruction about it here: https://github.com/crsf-wg/crsf/wiki/Message-Format#sync

Sounds good then :)) I'll merge the PR.

@tact1m4n3
Copy link
Owner

tact1m4n3 commented Jan 11, 2025

But I can do it as an error, too. In that case I would probably want that error to contain raw packet bytes, because what I work on is a proxy program which takes CRSF stream from RF module's UART, looks for interesting bits, and then sends it to flight controller UART anyway, so such RC data can't be discarded.

I'll probably add some sort of error for this anyway in the future.
On second thought I don't think an error is a good idea :)) I'd rather have the parser automatically sync to the first valid packet.

@tact1m4n3
Copy link
Owner

Thank you so much for your contribution!

@tact1m4n3 tact1m4n3 merged commit b94bc9d into tact1m4n3:main Jan 11, 2025
2 checks passed
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