Rename endian.h to portable_endian.h

[urrsk]

This change renames the endian.h file to portable_endian.h to avoid conflicts caused by circular includes. The third-party endian.h file includes the system's endian.h, which can lead to circular imports due to overlapping filenames. By renaming the file, we eliminate this issue and ensure compatibility in the CMake build process.

Finally, a macOS CI build has been added to verify that the library builds successfully on macOS, ensuring cross-platform compatibility.

---

Note

Medium Risk
Updates the project-wide endian conversion include and removes the previously-installed 3rdparty/endian header, which could break downstream builds that referenced the old path/name. While intended to be a no-behavior-change portability fix, it touches binary parsing/serialization codepaths and cross-platform build configuration.

Overview
Resolves endian.h include-name conflicts by replacing the vendored 3rdparty/endian/endian.h with a library-owned include/ur_client_library/portable_endian.h, and switches call sites to include that header instead of <endian.h>.

CMake install/include wiring for the old third-party header is removed, and CI is expanded with a new macOS (arm64 + x86_64) build/test workflow plus scheduled runs for Windows. Tests are strengthened/adjusted around BinParser and PackageSerializer endian encode/decode behavior (additional coverage and corrected assertions for int/bool/double cases).

^{Written by Cursor Bugbot for commit e8522d5. This will update automatically on new commits. Configure here.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.38%. Comparing base (59db585) to head (e8522d5).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #442      +/-   ##
==========================================
+ Coverage   75.70%   76.38%   +0.67%     
==========================================
  Files         105      105              
  Lines        5536     5542       +6     
  Branches      593      593              
==========================================
+ Hits         4191     4233      +42     
+ Misses       1050     1011      -39     
- Partials      295      298       +3     

Flag	Coverage Δ
start_ursim	82.41% <ø> (-1.89%)	⬇️
ur20-latest	68.81% <ø> (+0.09%)	⬆️
ur5-3.14.3	71.94% <ø> (?)
ur5e-10.11.0	65.29% <ø> (+0.23%)	⬆️
ur5e-10.12.0	65.98% <ø> (-0.14%)	⬇️
ur5e-10.7.0	64.22% <ø> (+0.13%)	⬆️
ur5e-5.9.4	72.51% <ø> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[urfeex]

Looking good in general, but I don't understand why so many implementation files now need to include that header, as well as the tests. Shouldn't that be unnecessary, when only the serializer, parser and package header cpp files include it?

[gavanderhoorn]

Just a casual observer (and observation): byte swapping is often kept header only (in C/C++) to give the compiler maximum opportunity to in-line it (which can result in swaps becoming no-ops when source and target use identical order). Refactoring it into a separate shared library seems to complicate that -- unless something like LTO is used (but I'm not sure that's possible with functions from shared libraries).

Performance is probably not necessarily a concern here, but something to keep in mind perhaps (at 500 Hz there could be measurable effects).

[urrsk]

Just a casual observer (and observation): byte swapping is often kept header only (in C/C++) to give the compiler maximum opportunity to in-line it (which can result in swaps becoming no-ops when source and target use identical order). Refactoring it into a separate shared library seems to complicate that -- unless something like LTO is used (but I'm not sure that's possible with functions from shared libraries).

Performance is probably not necessarily a concern here, but something to keep in mind perhaps (at 500 Hz there could be measurable effects).

Thanks @gavanderhoorn for that point.
The motivation was to make it easier to use the library and not need to handle header dependencies that is not used in the main header

[gavanderhoorn]

@urrsk wrote:

The motivation was to make it easier to use the library and not need to handle header dependencies that is not used in the main header

but in order to be able to in-line, that header is used, right?

Is the main issue just to have to install a .h in the 3rdparty directory?

[urrsk]

@urrsk wrote:

The motivation was to make it easier to use the library and not need to handle header dependencies that is not used in the main header

but in order to be able to in-line, that header is used, right?

Is the main issue just to have to install a .h in the 3rdparty directory?

Well yes. Though this work was motivated to avoid the circular include due to the naming, and then I when all the way to avoid it in the headers to limit the overhead.
So maybe it is worth just including the header as well.

[urrsk]

I have changed the scope and reverted the cpp files approach. Thanks for the inspiration from @gavanderhoorn.
Though, kept the approach of renaming the endian.h file to avoid circular includes and simplify the CMake setup.

[urfeex]

@traversaro, @acmorrow Would you happen to have an opinion on this change?

[traversaro]

@traversaro, @acmorrow Would you happen to have an opinion on this change?

This change is a really good idea! Did you considered also changing the install path from endian\portable_endian.h to something less collision-prone like ur_client_library\portable_endian.h ? Also other aspects of the header are collision prone (the header guard macro, and in general all the macros that are not namespaced and are defined without checking if they existing before, but probably changing those is out of scope).

[acmorrow]

@traversaro, @acmorrow Would you happen to have an opinion on this change?

This change is a really good idea! Did you considered also changing the install path from endian\portable_endian.h to something less collision-prone like ur_client_library\portable_endian.h ? Also other aspects of the header are collision prone (the header guard macro, and in general all the macros that are not namespaced and are defined without checking if they existing before, but probably changing those is out of scope).

I agree with the above. This seems like a solid improvement. I would recommend maybe looking for another source of endian conversion functions in the long term. While boost and abseil both provide such, it does seem from a quick look at the CMakeLists.txt that this library is attempting to have few if any heavyweight and/or non-vendored third-party dependencies, and that's a choice I can appreciate. But at least with C++20 and C++23 you have std::byteswap and std::endian. You could write a small non-macro based header that provides a polyfill under those names, or find one.

[urfeex]

Thank you both @traversaro @acmorrow for your prompt and valuable feedback!

[urrsk]

@traversaro thanks for the feedback. With my last commit, I have tried to implement your suggestions.
@acmorrow you also broad some good suggestion, though C++20 is not yet a common standard for everybody, therefore I would like to postpone this to the future. Though, it sounds very nice that std:: now also have addressed this.

You both more than welcome to common my changes. The goal is to make it easier to integrate and use.

[traversaro]

Great, thanks!