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

full commander and safety logging by default #15355

Merged
merged 3 commits into from
Jul 17, 2020

Conversation

dagar
Copy link
Member

@dagar dagar commented Jul 16, 2020

For debugging if something goes wrong.

@dagar
Copy link
Member Author

dagar commented Jul 16, 2020

It looks like this would increase the typical nominal logging rate by somewhere around half a kilobyte per second with a corresponding (tiny) increase in cpu.

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Makes sense.

@dagar dagar force-pushed the pr-commander_and_safety_logging branch from ed31e2d to ad90f46 Compare July 17, 2020 14:04
@dagar dagar requested a review from bresch July 17, 2020 14:04
@dagar
Copy link
Member Author

dagar commented Jul 17, 2020

Here we go again.
Screenshot from 2020-07-17 10-30-52

@dagar dagar merged commit 42493b3 into master Jul 17, 2020
@dagar dagar deleted the pr-commander_and_safety_logging branch July 17, 2020 21:27
@bkueng
Copy link
Member

bkueng commented Jul 20, 2020

Why not adding the arming/disarming reason to vehicle_status? That would make it perfectly clear what the reason was.

@dagar
Copy link
Member Author

dagar commented Jul 20, 2020

Why not adding the arming/disarming reason to vehicle_status? That would make it perfectly clear what the reason was.

I'm certainly not opposed to that. Explicit state transition reasons is something I'm hoping can be part of any state machine library/framework/etc adoption. #10584

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

Successfully merging this pull request may close these issues.

4 participants