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

Fix build for Ubuntu 24.04 #30

Closed
wants to merge 1 commit into from

Conversation

ruffsl
Copy link

@ruffsl ruffsl commented May 29, 2024

When building from source for ROS 2 Jazzy via Ubuntu 24.04, one may encounter undefined uint8_t types:

Starting >>> sick_safetyscanners_base
--- stderr: sick_safetyscanners_base                             
In file included from /opt/overlay_ws/src/vendor/SICKAG/sick_safetyscanners_base/src/datastructure/DeviceStatus.cpp:35:
/opt/overlay_ws/src/vendor/SICKAG/sick_safetyscanners_base/include/sick_safetyscanners_base/datastructure/DeviceStatus.h:72:3: error: ‘uint8_t’ does not name a type
   72 |   uint8_t getDeviceStatus() const;
      |   ^~~~~~~
/opt/overlay_ws/src/vendor/SICKAG/sick_safetyscanners_base/include/sick_safetyscanners_base/datastructure/DeviceStatus.h:39:1: note: ‘uint8_t’ is defined in header ‘<cstdint>’; did you forget to ‘#include <cstdint>’?
   38 | #include <iostream>
  +++ |+#include <cstdint>
   39 | 

The difference in behavior between GCC 11.4.0 and GCC 13.2.0 may likely be due to changes in the C++ standard version used by default by the compiler, or changes in the implementation of the standard library. I think in C++11 and onwards, uint8_t is defined in the header. However, some older versions of GCC, or some configurations, might have allowed uint8_t to be used without explicitly including , possibly because another standard library header included indirectly?

In GCC 13.2.0, it seems that this is no longer the case, and we need to explicitly include to use uint8_t. But perhaps this is more in line with the C++ standard, which requires explicit inclusion of the headers defining the types used.

@ruffsl ruffsl marked this pull request as ready for review May 29, 2024 20:42
@ruffsl
Copy link
Author

ruffsl commented May 29, 2024

Oh, possible duplicate of this, although I didn't seem to need to add #include <string> for building locally.

cc @carlatcrown

@carlatcrown
Copy link
Contributor

Oh, possible duplicate of this, although I didn't seem to need to add #include <string> for building locally.

cc @carlatcrown

Yes - same base cause - Noble updates :). We are iwyu so the was probably a cppcheck info warning (haven't double checked).

Not sure any owners are monitoring this repo currently - my PR was from Feb, might need to go through the company rep process for a prod etc.

@ruffsl
Copy link
Author

ruffsl commented May 29, 2024

@puck-fzi , please see this fix to help with your release into Jazzy:

Sakura286 added a commit to revyos-ros/sick_safetyscanners_base-release that referenced this pull request Jun 7, 2024
* Add compile option -Wno-error=pessimizing-move
* Add <cstdint> header

SICKAG/sick_safetyscanners_base#30

Signed-off-by: CHEN Xuan <[email protected]>
Sakura286 added a commit to revyos-ros/sick_safetyscanners_base-release that referenced this pull request Jun 7, 2024
* Add compile option -Wno-error=pessimizing-move
* Add <cstdint> header

SICKAG/sick_safetyscanners_base#30

Signed-off-by: CHEN Xuan <[email protected]>
Sakura286 added a commit to revyos-ros/sick_safetyscanners_base-release that referenced this pull request Jun 7, 2024
* Add compile option -Wno-error=pessimizing-move
* Add <cstdint> header

SICKAG/sick_safetyscanners_base#30

Signed-off-by: CHEN Xuan <[email protected]>
@lenpuc
Copy link
Collaborator

lenpuc commented Jun 19, 2024

Closing since #25 was merged

@lenpuc lenpuc closed this Jun 19, 2024
Z572 pushed a commit to revyos-ros/sick_safetyscanners_base-release that referenced this pull request Sep 2, 2024
* Add compile option -Wno-error=pessimizing-move
* Add <cstdint> header

SICKAG/sick_safetyscanners_base#30

Signed-off-by: CHEN Xuan <[email protected]>
Z572 pushed a commit to revyos-ros/sick_safetyscanners_base-release that referenced this pull request Sep 23, 2024
* Add compile option -Wno-error=pessimizing-move
* Add <cstdint> header

SICKAG/sick_safetyscanners_base#30

Signed-off-by: CHEN Xuan <[email protected]>
Z572 pushed a commit to revyos-ros/sick_safetyscanners_base-release that referenced this pull request Sep 23, 2024
* Add compile option -Wno-error=pessimizing-move
* Add <cstdint> header

SICKAG/sick_safetyscanners_base#30

Signed-off-by: CHEN Xuan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants