search

linuxkidd / rvc-monitor-py

RV-C Monitor - Python Edition
Apache License 2.0
17 stars 10 forks source link

Add a few more code comments/documentation #5

Closed spbrogan closed 2 years ago

spbrogan commented 2 years ago

Any chance the authors would be willing to document a little more of the code and process.

  1. Can you explain what you are doing in "convert_unit" function? I added Pa for pascals but not sure if i need to do anything here?
  2. Can you explain a little more about MQTT layout for sending/transmitting?
  3. What is the parameterize_string feature all about?

Thanks

linuxkidd commented 2 years ago

Hi @spbrogan Thanks for your interest in the project! :)

  1. RV-C has specific ways it represents different types of numeric data. The data representation depends on if the value is 8bit or 16bit, and what unit the data has ( percent, amps, volts etc ). The conversion is documented in the RVC Spec in table 5.3 starting on page 38. The convert_units function handles conversion from raw integer into the proper output base 10 value. -- If there's a pressure designation in the RV-C spec, you could certainly add it in the convert_units function, but otherwise, it wouldn't really be used. I suspect you'd really want to create your own converter function, similar to tempC2F

  2. The MQTT to CAN functionality is currently commented out. By default, the code subscribes to RVC/transmit and expects the MQTT message contents to be json formatted, something like { 'canid': 0x1234abc, 'payload': 0x351431 } (not actual values, and even the commented line isn't properly formatted to handle this.. it's more of a left-over from another of my projects where I copied the MQTT code from ).

  3. Parameterize_string function converts the fully spelled out parameter name into something easier for json parsing. e.g. it converts spaces, / and ()'s to underscore. ( for what it's worth, this function does have a decent comment above it explaining what it does.. :D )

Hope this helps!

spbrogan commented 2 years ago

thanks for the update.
I have a few more questions if you are willing. :)

  1. It doesn't look like the spec yaml and processing loop have any concept of messages that are not simple DGNs. For example, information requests, NAK/ACK messages (rv-c 3.2.4.3 and 3.2.4.4)? Am I missing something?

  2. I see some depreciated messages from the yaml. Is there something special about "ruamel.yaml". I have always used pyyaml but not really sure why it is important the parsed yaml has comments?

PendingDeprecationWarning: round_trip_load_all will be removed, use

yaml=YAML() yaml.load(...)

instead

Again - I appreciate the info. I may start a derivative project because i want to refactor a few things but i am really curious your plans and investments for this project.

linuxkidd commented 2 years ago

Sure, always willing to help!

  1. The status / control functions which were important to CoachProxy ( and Home-Assistant ) don't really need to worry with NAK/ACK messages. So, it never got much attention in code.

  2. We opted to use ruamel.yaml for the main reason that it can take On, Off, True, False, and make them a string for outputting into the JSON format. I found that there were decoding oddities that ended up being due to oddities in how the base yaml package handles booleans.

    • If there's a way to accomplish this in pyyaml, or any other package that has some other benefit... I'm certainly open to changing.

Re: Plans, see my reply in the other open issue. (Oh, and discussions are now enabled :D )

spbrogan commented 2 years ago

Thanks for the info. I'll close this and move future questions/discussions to the discussion forum.