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

Mission Log #10738

Merged
merged 14 commits into from
Oct 26, 2018
Merged

Mission Log #10738

merged 14 commits into from
Oct 26, 2018

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Oct 22, 2018

This adds a new log file to the SD card configurable via SDLOG_MISSION parameter.

Use-cases

It's also a ULog file, but only contains minimal information for the following use-cases:

  • geotagging/survey (more generally: extract data collected by the pixhawk during a mission and needed for post-processing)
  • cloud upload & statistics: keeping track of where and how long a vehicle has flown, and if there were problems.
  • regulation: keeping a small log to show where a drone was flown, what failures happened etc.

It is stored in another directory (mission_log) to keep the logs separate from the normal flight logs.

@dogmaphobic This will need an interface update on the QGC side, and maybe changes/extensions to the log download. I have not looked into that yet.

Implementation

  • the general structure of the logger is the same, but the mission logs use a separate independent write buffer.
  • optimization to reduce the header size: now only the message formats are written for messages that are actually logged. Reduces the header size by about 13KB for a normal log.
  • due to other optimizations, RAM usage does not increase if the mission log is disabled.
    If enabled, only the additional buffer of 300 bytes is required.

Testing

I ran a stress-test on a low-quality SD card for 1h:30min with 90KB/s normal logging rate and enabled mission log with 5Hz camera capture publication. There were no drops for the mission log, and the maximum used buffer was 200 bytes, so 300 bytes buffer size will be enough.
CPU load minimally increased from 5.6% + 1.7% to 5.7% + 1.8% for the main and writer threads.

Fixes #10189.

I enabled the mission log by default so that we get it field-tested. But it does not have to be enabled later on.

Previously the formats of all known uorb messages were written.

- reduces header size by about 13KB
- reduce ulog_message_format_s size to reduce required stack size.
  Largest message format is about 1000 bytes.
Not used yet, it should not affect anything, except for slight RAM
increase.
256 subscriptions are enough for now.
Reduces RAM usage by 300 bytes.
Some message formats are longer than the 300 bytes. We can split the writes
because we have to wait until they are written anyway.
- mission logs are stored in a separate directory mission_log
- It's disabled by default
- Does not increase RAM usage if disabled (if enabled, only 300 bytes)
- Log rotate does not apply to the mission logs
To get it field-tested. This can be reverted for a release...
/**
* Mission Log
*
* If enabled, a second mission log file will be written to the SD card.
Copy link
Contributor

@hamishwillee hamishwillee Oct 23, 2018

Choose a reason for hiding this comment

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

HOw about:

/**
 * Mission Log
 *
 * If enabled, a small additional "mission" log file will be written to the SD card.
 * The log contains just those messages that are useful for tasks like 
 * generating flight statistics and geotagging.
 *
 * The different modes can be used to further reduce the logged data
 * (and thus the log file size). For example, choose geotagging mode to
 * only log data required for geotagging.

 * Note that the normal/full log is still created, and contains all
 * the data in the mission log (and more).
 */

The @value 1 Complete was also a little confusing to me when I first saw it, because I thought maybe this meant "full log", rather than just "Complete mission log".
Maybe it is ok. Alternative:

 * @value 0 Disabled
 * @value 1 All mission messages
 * @value 2 Geotagging mission messages

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, updated accordingly.

@hamishwillee
Copy link
Contributor

Looks cool. I commented on the parameter doc.

Can we also update the user guide with this and also cover the other SDLOG_ params? Proposal is that this should be added in user guide https://docs.px4.io/en/getting_started/flight_reporting.html

  • Add a section on "Configuring logging" covering all the params
  • Explain which logs can be used for what - for example, is Mission Log useful in Flight Reporting?

I thought it would be good to add a link to the geotagging log in geotagging docs, but we don't have any. Do we need a page that covers what we offer in this area? If so, what would such a page offer?

Last of all, I looked at the devguide logging topic https://dev.px4.io/en/log/logging.html
My guess is that the logger is set up with all the rates information etc when the board is set up - and the user guide people don't need to know about this?
Should we perhaps just add a note in that logging topic about the SDLOG parameters.

@bkueng
Copy link
Member Author

bkueng commented Oct 23, 2018

Can we also update the user guide with this and also cover the other SDLOG_ params? Proposal is that this should be added in user guide https://docs.px4.io/en/getting_started/flight_reporting.html

Ok.

Explain which logs can be used for what - for example, is Mission Log useful in Flight Reporting?

No.

I thought it would be good to add a link to the geotagging log in geotagging docs, but we don't have any. Do we need a page that covers what we offer in this area? If so, what would such a page offer?

Yes we should have that. But I don't know of anything that exists already either.

My guess is that the logger is set up with all the rates information etc when the board is set up - and the user guide people don't need to know about this?

You generally don't have to change anything, since the defaults are good for general log analysis. Special use-cases that differ from that include:

  • ekf2 replay
  • enabling the high-rate logging profile for PID & filter tuning
  • if you want your own set of logged topics.
  • thermal temperature calibration

@hamishwillee
Copy link
Contributor

Thanks @bkueng . I'll look into updates to those docs once this is merged.

@bkueng
Copy link
Member Author

bkueng commented Oct 24, 2018

CI fails:

remote file operation failed: /tmp/jenkins/workspace/irmware-compile_mission_log-VMOKXZB2XWPNUMKC2VMXOFD4RIUCZ46BF3YVCQJRPP6QGICMO3JA@tmp/durable-cb023d6c at hudson.remoting.Channel@61b65624:ec2_docker_slave (i-0001bbab6136cffbf): java.nio.file.FileSystemException: /tmp/jenkins/workspace/irmware-compile_mission_log-VMOKXZB2XWPNUMKC2VMXOFD4RIUCZ46BF3YVCQJRPP6QGICMO3JA@tmp/durable-cb023d6c: No space left on device

@dagar can you look into it?

@dagar
Copy link
Member

dagar commented Oct 24, 2018

@bkueng we had a zombie ec2 slave. I'll restart these jobs now.

)

param_t _handle_back_trans_dec_mss{PARAM_INVALID};
param_t _handle_reverse_delay{PARAM_INVALID};
float _param_back_trans_dec_mss{0.f};
Copy link
Member

Choose a reason for hiding this comment

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

Picky minor side note - we should have some basic float literal guidelines. Inconsistent use of 0.f, 0.0f, 0.0F throughout the codebase should be an easy eyesore to avoid.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. I have no problem overlooking those though.

add_mission_topic("camera_capture");
add_mission_topic("mission_result");
add_mission_topic("vehicle_global_position", 1000);
add_mission_topic("vehicle_status", 1000);
Copy link
Member

Choose a reason for hiding this comment

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

vehicle_status is published at 1Hz or when something changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I saw that, but I wanted to be explicit/conservative here in case something changes in commander.

@dagar
Copy link
Member

dagar commented Oct 24, 2018

Some may want vehicle_attitude as well. Pix4d can optionally use roll, pitch, yaw.

Nevermind, just reviewed the contents of camera_capture.

@Avysuarez
Copy link

I tested in pixhawk 4 and pixhawk 4 mini in both created a folder with the name of mission_log.
pixhawk 4 log
https://review.px4.io/plot_app?log=eaed8008-87c9-40a3-b209-d23c50208f5f
pixhawk 4 mini log
https://review.px4.io/plot_app?log=f074290a-4f59-4cff-9a05-23adf99b21ef

@hamishwillee
Copy link
Contributor

@bkueng OK, docs for this waiting in PX4/PX4-user_guide#372 . Note that they are only peripherally about the mission log - without a topic about something that uses it (e.g. geofencing) there isn't much to say about it yet. So consider this "a start".

@bkueng bkueng merged commit dc62454 into master Oct 26, 2018
@bkueng bkueng deleted the mission_log branch October 26, 2018 06:02
@bkueng bkueng mentioned this pull request Oct 26, 2018
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