Improve SBP protocol PDF documentation [INFRA-280] #844
Conversation
There was a problem hiding this comment.
It looks like the new public field caused the messages to be excluded from the Python SBP message table, so a unit test failed.
I don't think we should rename the Linux messages, this will cause build failures in dependent libraries piksi_buildroot_private. Instead we can just drop the Linux resource monitor messages (basically everything in linux.yaml) from the docs since the feature was never officially released.
Otherwise, we'll need a companion change for PBRP, see resource_monitor code for an example.
| short_desc: Flags for a given GNSS sensor used as input for the fuzed solution. | ||
| short_desc: Instruments the physical type of GNSS sensor input to the fuzed solution. | ||
| public: true | ||
| description_only: true |
There was a problem hiding this comment.
How about embedded_type: true or generate_docs: false?
There was a problem hiding this comment.
Thanks so much for the review Jason.
Yes I agree now that renaming linux is bad. I will drop those messages from PDF for now instead.
I like "embedded_type" a lot and will go with that instead. I couldn't think of a good name so went with an obviously bad one hoping a better one would come to me.
The unit tests are wrong since they call a message a message that isn't, so I will update them.
cc @jaredw42 |
…e structure that isn't a message
…er we have a message or not
… Add embedded_type flag.
denniszollo
force-pushed
the
dzollo/improve_pdf
branch
from
05ca16a to
09dbf5f
Compare
* change pdf column widths to avoid text running into itself * Reduce width of overrunning table of messages in pdf * pdf: change width of last column long table of messages * pdf: make underscores searchable at the expense of underscores in line with text
… messages in python test
denniszollo
force-pushed
the
dzollo/improve_pdf
branch
from
09dbf5f to
1edfec3
Compare
| - MSG_LINUX_CPU_STATE: | ||
| id: 0x7F00 | ||
| short_desc: List CPU state on the system | ||
| public: false |
There was a problem hiding this comment.
One last tweak... there seems to be some cognitive dissonance between the meaning of "public" at the top level of the module and here on an individual message. For the top level it just puts the message in a different location in the docs, for the message level it removes it from the docs. To resolve this I would suggest we rename this to "hidden" and invert the logic, so public: false would become hidden: true.
There was a problem hiding this comment.
that is a good idea, but I think I'd like to do a separate PR that does that all at once if this looks good as is.
There was a problem hiding this comment.
Ok, logged a new ticket to track: https://swift-nav.atlassian.net/browse/INFRA-281
There was a problem hiding this comment.
I am thinking "publish:" (same logic) or "hide_from_docs:" (opposite logic).
silverjam
changed the title
I made quite a few changes to try to get an improved PDF document.