search

bosch-engineering / off_highway_sensor_drivers

This project provides ROS drivers for Bosch Off-Highway sensor systems. (Radar Off-Highway Premium, Radar OHW, General Purpose Radar Off-Highway (GPR), Ultrasonic Sensor System OHW Entry & Premium)
Other
48 stars 13 forks source link

Small fixes #4

Closed Rayman closed 5 months ago

Rayman commented 6 months ago

Hello,

I've fixed four small issues while tring to compile this package with clang

  1. Properly initialize the parent object of message_counter
  2. -pedantic doesn't work on clang
  3. Add proper include for spin_some
  4. Remove unused variable
Rayman commented 6 months ago

Hello, can I get some feedback on my PR?

daniel4ohw commented 6 months ago

Hello @Rayman
sorry for not replying sooner. First of all, thank you for your contribution! We are working on other issues in parallel and we have not had a chance to analyze it yet. I hope you will appreciate this. I'll get back to you as soon as possible.

TimStricker commented 6 months ago

Just FYI: I've had the same issue with the message counter (-Wmissing-braces) on the noetic-devel branch. Also in the off_highway_radar and off_highway_general_purpose_radar.

-Wpedantic worked for me though, using clang 15.0.7.

Rayman commented 6 months ago

I compiled with clang 15. If I turn on -pedantic I get errors from PCL, which I can't fix here

Rayman commented 5 months ago

@daniel4ohw did you have time to take a look at this PR? It's only a few minor changes and should be reviewable in a minute

daniel4ohw commented 5 months ago

@Rayman Thank you for your request. We are currently working on other topics that will also appear in the repo soon. We have planned this PR for next week.

Rayman commented 5 months ago

Thank you for the reply!

rcp1-beg commented 5 months ago

@Rayman Can you sign your commits with a GPG / SSH key? Although not stated anywhere yet, I would like to enforce it in this repo. Thanks!

Rayman commented 5 months ago

Done! I've never signed my commits before, so hopefully this works

daniel4ohw commented 5 months ago

Hi @Rayman unfortunately we will not be able to finish the contribution of the commit this week due to illness. But good news - we have made further adjustments to make it compatible with Clang (at least for now). The update will be available soon.. We still have some content-related issues ahead of us, so we are not focusing on supporting Clang for the next few months. If you have any problems just let us know.

rcp1-beg commented 5 months ago

Hi @Rayman,

we've just pushed our fixes for Clang. I took some of your commits over but did not remove the pedantic warnings in general and rather removed only the problematic warnings for GNU extensions and anonymous structs. Please check if 0.5.1 works for you with Clang.

If 0.5.1 works for you, I'll close this PR.

Also, I've dropped the commit GPG signing for the moment due to limitations of GitHub. Sorry about your wasted effort!

Rayman commented 5 months ago

0.5.1 seems to compile on my PC with clang. Thank you! I'll close this PR then.

TimStricker commented 5 months ago

@rcp1-beg Are you going to merge these fixes for noetic-devel as well? If not planned, I could set up a PR because I already did it on my fork anyway.

daniel4ohw commented 5 months ago

Hello @TimStricker we want to kept the noetic branch inactive due to numerous updates on the humble branch. However, if you'd like, you're welcome to publish your private fork. Thanks for your efforts! Daniel