Skip to content
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

MAVExplorer: Update lookups for EV & ERR messages #1320

Merged
merged 1 commit into from
Feb 10, 2024

Conversation

shancock884
Copy link
Contributor

This PR updates the MAVExplorer lookup tables for events (as used on EV messages) and subsystems (as used on ERR messages), to align with the latest list in AP_Logger.h.
I have also re-worked the lookup table of subsystem-specific error codes, to allow them to be included when running the 'messages' command. Note that the previous list was unused, and only the number was shown.
The list now uses the subsystem name (with optional wildcard) to select the relevant set of susbsytem-specific codes, falling through to the generic ones if necessary, and showing the number if all else fails.

Example output from testing with ArduSub-ArmFeatures-00000003.BIN from Autotest:

2023-12-30 09:59:22.779 Error: Subsys COMPASS ECode ERROR_RESOLVED 
2023-12-30 09:59:23.012 Error: Subsys FAILSAFE_LEAK ECode FAILSAFE_OCCURRED 
2023-12-30 09:59:24.009 Error: Subsys FAILSAFE_LEAK ECode FAILSAFE_RESOLVED 

The difference is that previously the text following ECode would have been just a number.

@peterbarker
Copy link
Contributor

The difference is that previously the text following ECode would have been just a number.

Problem is that sometimes this is just a number:
image

I'm not sure it was like that when I got here, but I'm definitely pointing the finger at someone else for that effort ;-)

Lookups for subsystems and events aligned to AP_Logger.h.

Reworked lookup table for subsystem-specific codes, so that they can be displayed.
Some have specific messages, some need to continue to display number (EKF_PRIMARY,
FLIGHT_MODE, FAILSAFE_FENCE), and others use generic codes.
@shancock884 shancock884 force-pushed the mavexplorer-ev-err-update branch from 16162ea to 859860c Compare February 10, 2024 09:26
@shancock884
Copy link
Contributor Author

Problem is that sometimes this is just a number:

Ah, that may scupper this cunning plan!

3 options I can see:

  • I remove the subsys-specific error codes bit, and we just update the events and subsystems in this PR.
  • Remove the fall-through/wildcard logic, so that error codes will be decoded only for those subsystems explicitly defined.
  • Add items to lookup table, so that those subsystems which should show a number do so (code search seems to suggest that this is only FAILSAFE_FENCE, EKF_PRIMARY and FLIGHT_MODE).

The last force-push does the 3rd option, by having things like this in the table:

    "EKF_PRIMARY" : { # EKF primary - code is the EKF number
        '*': "EKF:#",
    },

But thinking about it, 2nd option may be safer, in case someone adds a new subsystem in the future which needs to show numbers, but by default will show the generic code strings.

@shancock884
Copy link
Contributor Author

For info, the 3rd option would look like this - tested by manually adding some extra messages into the end of a DF text log:

1970-01-01 01:00:01.805 Error: Subsys FAILSAFE_FENCE ECode FAILSAFE_RESOLVED 
1970-01-01 01:00:01.805 Error: Subsys FAILSAFE_FENCE ECode Fence:1 
1970-01-01 01:00:01.805 Error: Subsys EKF_PRIMARY ECode EKF:0 
1970-01-01 01:00:01.805 Error: Subsys FLIGHT_MODE ECode Mode:0 
1970-01-01 01:00:01.805 Error: Subsys EKF_PRIMARY ECode EKF:1 
1970-01-01 01:00:01.805 Error: Subsys COMPASS ECode FAILED_TO_INITIALISE 
1970-01-01 01:00:01.805 Error: Subsys COMPASS ECode ERROR_RESOLVED

@peterbarker
Copy link
Contributor

Problem is that sometimes this is just a number:

Ah, that may scupper this cunning plan!

3 options I can see:

* I remove the subsys-specific error codes bit, and we just update the events and subsystems in this PR.

* Remove the fall-through/wildcard logic, so that error codes will be decoded _only_ for those subsystems explicitly defined.

* Add items to lookup table, so that those subsystems which should show a number do so (code search seems to suggest that this is only FAILSAFE_FENCE, EKF_PRIMARY and FLIGHT_MODE).

The last force-push does the 3rd option, by having things like this in the table:

    "EKF_PRIMARY" : { # EKF primary - code is the EKF number
        '*': "EKF:#",
    },

But thinking about it, 2nd option may be safer, in case someone adds a new subsystem in the future which needs to show numbers, but by default will show the generic code strings.

Well, there's option number four, which is, "fix ArduPilot" :-)

I might also mention that I do want o get down the log message metadata into MAVExplorer.py - that would allow us to do without this static list which gets out of date, and allow for msghelp UNIT to get the documentation on the UNIT log message. I've wanted this for years, mind you, and seem to have lost the start of an implementation I had.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@peterbarker peterbarker merged commit 6c1cf36 into ArduPilot:master Feb 10, 2024
2 checks passed
@peterbarker
Copy link
Contributor

Merged, thanks!

@shancock884 shancock884 deleted the mavexplorer-ev-err-update branch February 10, 2024 11:43
@shancock884
Copy link
Contributor Author

I might also mention that I do want o get down the log message metadata into MAVExplorer.py - that would allow us to do without this static list which gets out of date, and allow for msghelp UNIT to get the documentation on the UNIT log message. I've wanted this for years, mind you, and seem to have lost the start of an implementation I had.

Yes! That would be good! ... I'd been thinking about it too recently.... as well as the 2 things you mentioned (avoiding out-of-date hard-coded lists, and some sort of "msghelp" or "logmessage help" command), I had 2 other things in mind:

  1. Show enum and bitmask strings when doing "dump --verbose"
  2. Include message description (cut at 60 chars if necessary) in the stats output, e.g.:
ERR  0.00%  [Specifically coded error messages]
MODE 0.00%  [vehicle control mode information]
GPA  0.40%  [GPS accuracy information]

What I wasn't sure about was how the XML data gets in... auto-download when needed, user need to do "logmessage download" to get the extras, magic inclusion in pymavlink pip package....

@peterbarker
Copy link
Contributor

peterbarker commented Feb 10, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants