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
Touches cross-platform build configuration and endian conversion includes used in core parsing/serialization paths; behavior is likely unchanged but misconfigured include/install paths could break downstream builds.

Overview
Fixes endian header name collisions by renaming the bundled header to 3rdparty/endian/portable_endian.h and switching internal includes from <endian.h> to <endian/portable_endian.h>; CMake include/install rules are simplified so the endian headers are installed on all platforms under include/ur_client_library/3rdparty/endian.

CI coverage is expanded with a new mac-build GitHub Actions workflow (arm64 + x86_64) and both Windows and macOS workflows now run on a nightly schedule. Tests are tightened/expanded around BinParser/PackageSerializer endian encode/decode behavior and a few assertions are corrected (e.g., EXPECT_EQ vs EXPECT_DOUBLE_EQ, explicit void casts in EXPECT_THROW).

^{Written by Cursor Bugbot for commit 2b43f21. 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.43%. Comparing base (7b57b66) to head (e7b907f).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #442      +/-   ##
==========================================
+ Coverage   76.33%   76.43%   +0.09%     
==========================================
  Files         104      104              
  Lines        5531     5533       +2     
  Branches      594      592       -2     
==========================================
+ Hits         4222     4229       +7     
+ Misses       1010     1006       -4     
+ Partials      299      298       -1     

Flag	Coverage Δ
start_ursim	83.51% <ø> (+0.90%)	⬆️
ur20-latest	68.81% <ø> (+0.11%)	⬆️
ur5-3.14.3	71.84% <ø> (+0.03%)	⬆️
ur5e-10.11.0	64.97% <ø> (+0.26%)	⬆️
ur5e-10.12.0	65.94% <ø> (-0.25%)	⬇️
ur5e-10.7.0	64.23% <ø> (-0.19%)	⬇️
ur5e-5.9.4	72.49% <ø> (+0.01%)	⬆️

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!