-
Notifications
You must be signed in to change notification settings - Fork 597
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
DFReader.py: add bitmask data to verbose print output #937
Conversation
|
a4a1d70
to
2d42e64
Compare
Looks nice! A couple of thoughts you are free to consider or ignore :-) .... I know it is the same as dump_message_verbose in mavutil, but I don't really like the use of "!" for False values.... when I first came across it my eyes read it as a check-mark meaning true (i.e. opposite of what it means)! But maybe its best to remain consistent until someone has a strong enough argument/opinion to change both. I was thinking it could be good to split the dump_verbose_bitmask function into two: get_field_metadata and dump_verbose_bitmask, as it should make it easier to implement showing enums too in a later PR without looking up metadata twice (and probably needed to do following suggestion...). It would be nice to show the hex value for bitmasks, alongside the decimal (as dump_message_verbose does). If you do create get_field_metadata, then you can get the field metadata first and do something like |
You wouldn't be surprised to hear I find the ! thing readable, but definitely open to alternatives.
Seems reasonable. If you can put those suggestions in patch form I can apply them... otherwise I'll come back to this in a while. Or I could merge and accept patches later? |
@shancock884 I had another look at your suggestions. I've re-arranged things a little to be somewhat like what you're suggesting, but haven't modified the printing of the value. Too many cases to test for this PR ;-) Further patches would probably be something like |
2d42e64
to
ce519da
Compare
.. taking the conservative approach to this new feature failing....
@shancock884