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

system/uorb: require that LIBC_FLOATINGPOINT be enabled for DEBUG_UORB #2987

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

linguini1
Copy link
Contributor

@linguini1 linguini1 commented Feb 6, 2025

Summary

This pull request closes the issue raised here.

Now DEBUG_UORB depends on LIBC_FLOATINGPOINT. This is because it is not possible to view sensor debug output without this enabled, which is only a problem for systems without an FPU (systems with an FPU have this enabled by default).

Impact

Users now have to enable LIBC_FLOATINGPOINT on systems without an FPU in order to use DEBUG_UORB. The dependency marked with depends on means that the uORB debug option will not be visible until LIBC_FLOATINGPOINT is selected, but this was determined to be acceptable in the discussion under the linked issue.

This prevents the annoyance of re-flashing the device because floating point printing was disabled, rendering debug output almost useless.

I have also included a compile time error for the configuration mismatch.

Testing

Verified that enabling LIBC_FLOATINGPOINT caused DEBUG_UORB to appear and that the dependency was marked in Kconfig.

Output after building by selecting LIBC_FLOATINGPOINT followed by DEBUG_UORB in Kconfig.

nsh> uorb_listener

Mointor objects num:4
object_name:sensor_mag, object_instance:0
object_name:sensor_gyro, object_instance:0
object_name:sensor_baro, object_instance:0
object_name:sensor_accel, object_instance:0
sensor_mag(now:203370000):timestamp:203370000,x:-63.750000,y:-10.500000,z:-2280
sensor_accel(now:203380000):timestamp:203370000,x:0.047856,y:-0.196212,z:6.5380
sensor_baro(now:203390000):timestamp:203390000,pressure:496.140015,temperature0

Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thank you @linguini1 :-)

Please remember to provide test logs, and please update git commit message desription, like:

This fixes broken float in printf() like sensor debug messages
on systems without an FPU (systems with an FPU have this
enabled by default) by making DEBUG_UORB depend on
LIBC_FLOATINGPOINT. Solution dicussed and agreed in:
https://github.com/apache/nuttx/issues/15599.

@lupyuen
Copy link
Member

lupyuen commented Feb 6, 2025

@linguini1 Could you fill in the Description inside the Commit? As discussed in the Mailing List:
https://lists.apache.org/thread/mn4l1tmr6fj46o2y9vvrmfcrgyo48s5d

Thanks for all your awesome contributions! :-)

This change prevents users from trying to use the `uorb_listener`
application without having floating point printing enabled on systems
that do not have an FPU (systems with FPUs have `LIBC_FLOATINGPOINT`)
enabled by default. Solution dicussed and agreed in:
apache/nuttx#15599.
@linguini1
Copy link
Contributor Author

Thanks @lupyuen and @cederom! I had misunderstood the commit description to just be the short description, not a detailed one. I have updated the commit message and added testing logs. Let me know if anything else is required, I'll adhere to this better going forward!

@lupyuen
Copy link
Member

lupyuen commented Feb 6, 2025

Yep it looks great thanks! Quick Hack: Normally I copy the Description section of the PR into the Commit Description :-)

@raiden00pl raiden00pl merged commit fb0c1e1 into apache:master Feb 7, 2025
37 checks passed
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.

5 participants