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

uORB messages generated pretty print #9149

Merged
merged 9 commits into from
Mar 28, 2018
Merged

uORB messages generated pretty print #9149

merged 9 commits into from
Mar 28, 2018

Conversation

dagar
Copy link
Member

@dagar dagar commented Mar 23, 2018

This is an old branch I resurrected and didn't want to lose track of again. The idea is to create a print function for each uORB message at generation time. Currently the topic_listener does this with a separate python script (https://github.com/PX4/Firmware/blob/cc26c34691db1e622cddac6d330679fe61467d37/src/systemcmds/topic_listener/generate_listener.py), but has several small issues that are easily resolved when generating this properly at the source.

With a mechanism to print an arbitrary uORB message we can move listener over, and replace the many status and test messages throughout various drivers and modules. I'm also hopeful we can eventually fit this back into flash constrained devices. For example ekf2 status would grab and print the last copy of each published output message (estimator_status, etc). This largely eliminates the need for listener.

TODO

  • general review, restructuring, and cleanup

  • consider sharing the strings between the generated fields string and print helper
    eg vehicle_attitude

    constexpr char __orb_vehicle_attitude_fields[] = "uint64_t timestamp;float rollspeed;float pitchspeed;float yawspeed;float[4] q;float[4] delta_q_reset;uint8_t quat_reset_counter;uint8_t[3] _padding0;";``` void print_message(const vehicle_attitude_s& message)
    {
    printf("vehicle_attitude_s\n");
    printf("timestamp: %llu\n", message.timestamp);
    printf("rollspeed: %.3f\n", (double)message.rollspeed);
    printf("pitchspeed: %.3f\n", (double)message.pitchspeed);
    printf("yawspeed: %.3f\n", (double)message.yawspeed);
    printf("q: [%.3f, %.3f, %.3f, %.3f]\n", (double)message.q[0], (double)message.q[1], (double)message.q[2], (double)message.q[3]);
    printf("delta_q_reset: [%.3f, %.3f, %.3f, %.3f]\n", (double)message.delta_q_reset[0], (double)message.delta_q_reset[1], (double)message.delta_q_reset[2], (double)message.delta_q_reset[3]);
    printf("quat_reset_counter: %u\n", message.quat_reset_counter);
    }
    ```* ignore padding* update and simplify listener
    * command line syntax is horrible for specifying the number of messages or instance
    * ability to subscribe at a lower rate would be very helpful
    * listener -n <number of messages> -i <instance number> -r <rate> message
    * should make it clear if a message has never been published
  • consider adding pretty print options like...

    • time elapsed in seconds next to the timestamp in microseconds
      • eg timestamp: 12345678 (1.2 seconds ago)
    • euler angles next to a quaternion
    • decode some bitmasks?
    • boolean true/false
  • check all nested structures work

@dagar
Copy link
Member Author

dagar commented Mar 23, 2018

It might be worth pursuing this short term, it's already a ~ 2.5K flash savings on px4fmu-v2.

@dagar
Copy link
Member Author

dagar commented Mar 23, 2018

Adding timestamp context.

image

@dagar dagar force-pushed the pr-uorb_print branch 2 times, most recently from c304936 to 7b54cec Compare March 23, 2018 21:22
@dagar
Copy link
Member Author

dagar commented Mar 24, 2018

Updated listener to report if a topic has never been published, and it now works properly for position_setpoint_triplet (contains 3 position_setpoints).

All nested types should work.

@LorenzMeier
Copy link
Member

Nice!

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Cool! I have a few minor things, but it's definitely an improvement.

@@ -165,6 +179,70 @@ def convert_type(spec_type):
return c_type


def px4_printf(field):
Copy link
Member

Choose a reason for hiding this comment

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

I find the method name a bit confusing. How about print_field?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

"""

# skip padding
if ("_padding" in field.name):
Copy link
Member

Choose a reason for hiding this comment

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

only if it starts with padding: .startswith('_padding') (and drop the brackets)

Copy link
Member Author

Choose a reason for hiding this comment

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

done

void print_message(const @uorb_struct& message)
{
printf(" @(uorb_struct)\n");
printf("\ttimestamp: %" PRIu64 " (%.6f seconds ago)\n", message.timestamp, hrt_elapsed_time(&message.timestamp) / 1e6);
Copy link
Member

Choose a reason for hiding this comment

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

I'd add the (%.6f seconds ago) only if the timestamp is > 0

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -18,74 +18,33 @@
topics = []
message_elements = []

# large and not worth printing (find better solution)
raw_messes = [raw_messages.remove(x) for x in raw_messages if 'qshell_req' in x]
Copy link
Member

Choose a reason for hiding this comment

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

raw_messes = [x for x in raw_messes if not any(exception in x for exception in ['qshell_req', 'ulog_stream', ...])]

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@dagar dagar changed the title [WIP] uORB messages generated pretty print uORB messages generated pretty print Mar 27, 2018
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Good to go.
One thing we can consider is pretty-printing for strings. log_message currently prints as:

 log_message_s
        timestamp: 14282720 (9.598521 seconds ago)
        severity: 4
        text: [91, 109, 99, 95, 97, 116, 116, 95, 99, 111, 110, 116, 114, 111, 108, 93, 32, 97, 108, 114, 101, 97, 100, 121, 32, 114, 117, 110, 110, 105, 110, 103, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 80, 102, 200, 253, 253, 127, 0, 0, 32, 102, 200, 253, 253, 127, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 240, 101, 200, 253, 253, 127, 0, 0, 163, 60, 192, 8, 163, 127, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 216, 209, 129, 0, 0, 0, 0, 0, 80, 102, 200, 253, 253, 127, 0, 0]

@dagar dagar merged commit 416feea into master Mar 28, 2018
@dagar dagar deleted the pr-uorb_print branch March 28, 2018 13:15
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.

3 participants