-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Use correct enum size when generating tlm packet structures #4333
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
Use correct enum size when generating tlm packet structures #4333
Conversation
|
The CI errors are caused by a checkout of F Prime devel, not the current branch. |
|
@LeStarch Do you need anything more from me in order to get this PR merged? |
|
As you can see, the CI situation is not working. Need to determine what to do about it. |
Instead of hardcoding enums as 4 bytes, the enum size should be parsed from the topology XML.
85017d6 to
2ea624f
Compare
|
I just pushed a commit that should fix the CI issues, by manually specifying v3.6-compatible refs for the relevant repositories. All the actions ran successfully in my test branch, except for the two waiting for a Raspberry Pi to pick up the job. |
2ea624f to
9b0b4d6
Compare
|
I see 2 CI failures. The The |
9b0b4d6 to
cf21e0d
Compare
|
Might be one of the transient errors that occurred during this era of the cod base. |
|
@LeStarch those errors both look suspect to me, does poking them to re-run tend to work? We can't try that, I'm curious if theres a race condition that can be overcome by hitting the re-run button a few times. |
|
I cannot explain this RPI error, but I am of the belief we should merge over this objection. If it does not pass now, I will do this. |
7538242
into
nasa:release/v3.6.0-hotfixes
Change Description
This PR modifies the
tlm_packet_gen.pyautocoder to pull in the correct size of each enum type when generatingPacketsAc.cpp.Rationale
Prior to this change, the autocoder assumes a hardcoded 4-byte length for each enum channel, even though
TopologyAppDictionary.xmlandTopologyDictionary.jsonfiles are generated with the proper enum length. This leads to a discrepancy, where a GDS using the above Dictionary files will have a different packet definition than the fprime deployment is using to generate telemetry packets.This change brings the fprime telemetry packet generation process into consistency with the Dictionary files, at least where enum lengths are concerned.
Notes
The primary fix is to the
process_enum_files()function, which is modified to grab theserialize_typefrom the parsed XML viaenum_model, then evaluate that string (likely 'U8', 'U16', etc) usingget_type_size(), rather than assuming a hardcoded 4-byte length.A secondary fix is to remove what I believe to be dead code in other parts of the file. It looks like a past version of the autocoder stored enum types in a tuple rather than a string, so there are 3 instances (processing typical channels, processing structs/serializables, and processing arrays) where the type was being checked against
type(tuple())and inTruecases, the type was assumed to be an enum with size of 4 bytes. Since enum types are currently stored as strings, these 3 hardcodedifstatements never resolveTrue, so their hardcoded values don't really matter. But I've removed them for clarity.Testing/Review Recommendations
I've confirmed the following:
fprime-gdsnow happily displays packetized 1-byte, 2-byte, and 4-byte enum channelsFuture Work
There are some other hardcoded and possible out-of-date assumptions in this autocoder script that could be looked at, but it seems the autocoder was rewritten for F Prime 4.0, so there may be no need to improve this script further.
AI Usage (see policy)
N/A