Move xbus big-endian byte helper functions to common.h (#57) - #61
Move xbus big-endian byte helper functions to common.h (#57)#61gennaro-rescigno wants to merge 6 commits into
Conversation
|
Thank you for submitting the pull request! I notice that the comments you've included are not in English. As the members on our student design team communicate in English, If you could please update those comments I can have another look at the request. |
|
Hi @georgesleen, thank you for the feedback! I have updated the comment to English as requested and pushed the changes. Let me know if everything looks good now! |
|
Please translate all the comments so that I may review it. Thank you. |
|
Hi @georgesleen, I have reviewed both files, removed all the remaining Italian comments/notes, and made sure everything is strictly in English. Thanks again for your patience! |
| uint32_t(b[2]) << 8 | uint32_t(b[3])); | ||
| return std::bit_cast<float>(u); | ||
| } | ||
| // NOTE: The three big-endian functions that were here have been removed |
There was a problem hiding this comment.
Nit: This comment is not necessary and I would prefer it removed.
| for (std::size_t i = 0; i < N; i++) { | ||
| const auto id = write_u16_big_endian(static_cast<uint16_t>(items[i].id)); | ||
| const auto rate = write_u16_big_endian(items[i].rate_hz); | ||
| // Qualified with plrs:: prefix |
There was a problem hiding this comment.
This comment feels unnecessary to me.
| } | ||
| auto take = [&](std::size_t idx) { | ||
| return read_f32_big_endian( | ||
| // Qualified with plrs:: prefix |
There was a problem hiding this comment.
Again this comment is not needed.
| } | ||
|
|
||
| } // namespace xbus | ||
| } // namespace xbus No newline at end of file |
There was a problem hiding this comment.
Please leave newlines at the end of files.
georgesleen
added
enhancement
|
Closes #57 |
|
Additionally @gennaro-rescigno please complete the necessary formatting steps as required by this repository please. I have updated CONTRIBUTING.md to better explain the steps required to do this. When you push to your fork you are able to see the status of CI, please make sure that it passes for you. |
|
Hi @georgesleen, I have applied the official repository formatting rules to |
| return std::bit_cast<float>(u); | ||
| } | ||
|
|
||
| } // namespace plrs No newline at end of file |
There was a problem hiding this comment.
Please leave newlines at the end of files
|
@gennaro-rescigno please make sure that unit tests are passing for you locally so that I can merge your changes in. |
2 participants
Hi! This pull request addresses issue #57.
I have moved the xbus big-endian byte helper functions (
read_u16_big_endian,write_u16_big_endian, andread_f32_big_endian) fromxbus_protocol.hto the sharedcommon.hfile inside theplrsnamespace. I also updated the references inxbus_protocol.hto use theplrs::prefix.This is my very first open-source contribution, any feedback is highly appreciated!