-
Notifications
You must be signed in to change notification settings - Fork 4
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
Steven telemetry CAN autogen #343
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome thanks for doing this I think it generally looks pretty good. I think a quick unit test would make the usage a little clearer, plus we should think about integration a little bit before merging this. My only concern is the format of the input data, so we should see what the shape of our CAN frame is once we received it from the Xbee and have it in memory (e.g bitstream in an array, array of ints, etc.)
For now just add in a unit test (does not need to be rigorous at all) which should primarily serve as a usage example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, saw you working on this yesterday! Gonna close this out but it looks like you're missing the 103 files, 101 and 102 are here. Let me know if you need help with anything!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this file is autogenerated when im playing around with the generator.py to see how it generates files. If I accidentally deleted something it might unintentional.
for signal in message['signals']: | ||
print(f" - Name: {signal['name']}, Start Bit: {signal['start_bit']}, Length: {signal['length']}") | ||
|
||
return {"Boards": boards, "Messages": messages, "Messages_dict": messages_dict} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice you're right this should speed things up, not a big deal for our regular autogen but definitely useful for telemetry.
@@ -26,5 +26,4 @@ void can_tx_all() { | |||
(uint64_t) g_tx_struct.{{message.name}}_{{signal.name}} << {{signal.start_bit}}{{ " |" if not loop.last }} | |||
{%- endfor -%} | |||
); | |||
{%- endfor %} | |||
} | |||
{% } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the edit in the .c file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think when looking into the CAN message in C side and accidently changed something. Sorry about that.
{% set bitstream = data["bitstream"] -%} | ||
{% set messages_dict = data["Messages_dict"] -%} | ||
|
||
def decode_packet(messages_dict, bitstream): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here does the user pass in the messages dict themselves?
You're right that maybe a quick sample app/unit test would be good and answer this question implicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was also having a bit confusion in relation to this one. The jinja defines a python function that passes these two in as parameter, but currently they are located in generator.py. (data from get_data() that gets all the yaml files)
I am planning on after generating the python script, I just run python3 libraries/codegen/python_autogen.py
with some arbitrary defined CAN messages in the main function. Something similar to this:
Define message values
# source_id = 1 # source_id (3 + 4 bits)
# type_field = 0 # type (1 bit)
# msg_id = 3 # msg_id (24 bits)
# dlc = 5 Bytes # (40 bits)
# throttle_output_value = 30 # throttle_value (32 bits)
# brake_output_value = 31 # brake_value (8 bits)
# print(decode_packet(data["Messages_dict"], "0000 0001 0000 0000 0000 0000 0000 0011 0000 0000 0000 0000 0000 0101 0000 0000 0000 0000 0000 0000 0001 1110 0001 1111"))
If eventually this python_autogen.py file will be in libraries/codegen, I'm thinking about just doing import data from generator.py
to get the data array. Maybe I will look into this solution on Saturday in the bay.
|
||
def decode_packet(messages_dict, bitstream): | ||
# Clean the bitstream by removing spaces | ||
clean_bitstream = bitstream.replace(" ", "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, your function received a CAN frame right? Why are there spaces in the bitstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case something like 0000 0001 occurs, just removing the blank space in between.
Changes:
Questions: