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

Revamp yaml structure slightly #126

Merged
merged 24 commits into from
May 21, 2024
Merged

Revamp yaml structure slightly #126

merged 24 commits into from
May 21, 2024

Conversation

jr1221
Copy link
Contributor

@jr1221 jr1221 commented May 17, 2024

Reorganizes some of the yaml structure. Decoding only, I havent tested encoding.
List of changes:

  • Fixes endianness
  • Reorganizes yaml so the first level topic name is always the device the message originated from
  • Gets rid of compositefield and discretefield and replaces it with a two layer NetField/CANPoint configuration. This allows for composite fields with dynamic decoding sizes, etc. becuase it splits the parameters into decoding and mqtt sending sections.
  • Adds documentation to README on how to add messages.
  • Renames Mock to Calypso/Unknown
  • Fixes incorrect typing imports which broke the package on most distributions
  • Removes uneccessary format types, we will add them as they are needed
  • Adds a send flag, so some bits can be ignored
  • Adds a topic_append flag, so the first CANPoint can become part of the topic name at the end
  • Switches yaml parser to safe to pick up more syntax errors

Reasons this may not be usable

  • Probably breaks encoding with c templates
  • Adds a few extra lines to the yaml per message (points: -!CANPoint)
  • Uses the current way of formatting the code strings instead of templating

Things to add in future PRs, if this gets merged

  • Have calypso/unknown contain the message ID of the unknown message
  • Ability to set topic_append in the CANPoint, so as many things can be appending as needed instead of only one at the very beginning
  • Ability to set send in the CANPoint, so you can not send part of messages of 1+ points

@jr1221
Copy link
Contributor Author

jr1221 commented May 19, 2024

Additional changes

  • Switched to bitstream_io for parsing bits, as it has dynamic sizing, signed support, and little/big endian support. I was running into some isssues decoding unsigned integers as little endian, and stumbled upon this library. It is awesome because for the first time we can just specifiy what the bits are (big endian usually) without trying the architecture dependent swap bytes, from_be, etc. Has same cpu usage as our own BitReader.
  • For little endian, read a byte from the big endian stream as little endian. This allows for per-point endianness in the library. Defaults to big endian as that is most common on the can bus.

Additional verifications with this change

  • Verified little endian decoding
  • Verified signed unsigned conversion at 16 and 32 bits at big and little endian

See the README for the list of limitations in writing CAN messages. The biggest issue right now is signed messages must be full bytes, but tbh thats how everyone should be doing it imo.

Copy link
Contributor

@Peyton-McKee Peyton-McKee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean if it works it works. I personally feel like there must be a way to not have wrappers. But ig there was a reason i did it like that in the first place 🤷

cangen/Format.py Show resolved Hide resolved
cangen/README.md Outdated Show resolved Hide resolved
cangen/README.md Show resolved Hide resolved
cangen/README.md Show resolved Hide resolved
cangen/README.md Outdated Show resolved Hide resolved
cangen/RustSynth.py Outdated Show resolved Hide resolved
get rid of comment of removed field
@jr1221 jr1221 merged commit 22b868d into main May 21, 2024
1 check passed
@jr1221 jr1221 deleted the yaml-structure branch May 21, 2024 18:39
@jr1221 jr1221 mentioned this pull request May 21, 2024
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.

3 participants