Fixes for new warnings/behaviours in GCC7+/cppcheck.

ros-drivers / rosserial

A ROS client library for small, embedded devices, such as Arduino. See: http://wiki.ros.org/rosserial

508 stars 526 forks source link

Fixes for new warnings/behaviours in GCC7+/cppcheck. #480

Closed mikepurvis closed 3 years ago

mikepurvis commented 4 years ago

A bunch of stuff that came up when testing with the GCC9-based toolchain that will ship in Ubuntu Focal:

Add missing member initializations.
Add missing override specifiers.
Hacky cast for the 16 bit AVR float logic.

There are a few more member initializations to add in NodeHandle. Will get to those before merging.

Also, I believe this will break Bionic, and it's a pretty breaking change regardless, so this probably merits cutting a noetic-devel branch for it.

romainreignier commented 4 years ago

About improving code quality, it would be nice to make the time related functions const like I did in our qt version.

Also, we could align the current toNsec with the ROS toNSec (capital S). This is breaking the API but if a new noetic-devel branch is created, maybe it's the moment.

Regarding time functions, maybe we could try to use 64 bit variables even if I know that the major use case is on small microcontrollers so the time is not always an epoch time. What do you think about that?

mikepurvis commented 4 years ago

PR is up for noetic-devel branch PR testing here: https://github.com/ros/rosdistro/pull/24473

The tests will continue to fail until the Python 3 situation in rosserial_python is resolved. I might simply disable those tests in this PR so that it can merge and then open a separate one to get rosserial_python functional again.

Note also that the travis build will also have to be updated; there's no base VM for focal, so it should probably be switched to use the docker service, see: https://docs.travis-ci.com/user/docker/ (though that would be blocked on having the official ROS Noetic docker image— https://github.com/osrf/docker_images/pull/395)

mikepurvis commented 4 years ago

@stertingen Without proper Python 3 support landing for Noetic, the tests using rosserial_python don't pass. However, given the scope of the changes, perhaps it would make more sense to just disable those, merge the smaller original change here, and make a separate PR for fixing rosserial_python.

mikepurvis commented 3 years ago

Closed in favour of #508.