Conversation
fix:
```cpp
[ 57%] Building CXX object CMakeFiles/ffw_robot_manager.dir/src/battery_model.cpp.o
In file included from /build/ai_worker-release-release-jazzy-ffw_robot_manager-1.1.20-1/src/battery_model.cpp:17:
/build/ai_worker-release-release-jazzy-ffw_robot_manager-1.1.20-1/include/ffw_robot_manager/battery_model.hpp:68:11: error: 'uint8_t' does not name a type
68 | virtual uint8_t get_power_supply_technology() const = 0;
| ^~~~~~~
```
There was a problem hiding this comment.
Pull request overview
Fixes a GCC 15 build failure in ffw_robot_manager by ensuring the standard integer type uint8_t is available in the public battery model interface header.
Changes:
- Add the missing
<cstdint>include tobattery_model.hppsouint8_tis defined when the header is compiled.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request adds the header to battery_model.hpp to resolve compilation issues with fixed-width integer types. The feedback suggests ensuring that all files using these types include the header directly rather than relying on transitive includes and recommends using the std:: namespace for types like uint8_t for better C++ standard compliance.
| #ifndef FFW_ROBOT_MANAGER__BATTERY_MODEL_HPP_ | ||
| #define FFW_ROBOT_MANAGER__BATTERY_MODEL_HPP_ | ||
|
|
||
| #include <cstdint> |
There was a problem hiding this comment.
While adding <cstdint> correctly resolves the compilation error in this header, there are a few related improvements to consider for full GCC 15 compatibility and standard compliance:
- Direct Inclusions: Other files in the package also use fixed-width integer types without directly including
<cstdint>(e.g.,uint16_tinubetter_battery_model.hppanduint32_tinffw_robot_manager.cpp). Although they may currently receive the header transitively through this file, it is best practice for each file to include the headers it depends on directly. - Namespace Qualification: In C++, types from
<cstdint>are defined in thestdnamespace. It is recommended to usestd::uint8_t,std::uint16_t, etc., instead of the global namespace versions (e.g., at line 68 of this file) for better portability and adherence to the C++ standard.
|
Hi @nim65s |
fix: