[#498] cleanup message type details

[elfenpiff]

Notes for Reviewer

In this PR the API used when overriding the MessageTypeDetails is strictly separated from the end user API.

• When overriding the MessageTypeDetails you must use [CustomPayloadMarker] as message type
• You can only loan samples via Publisher::loan_custom_payload()
• You can only receive samples via Subscriber::receive_custom_payload().

The reason is, that in those cases the Sample and SampleMut handle the data a bit different. When calling Publisher::loan_custom_payload(123) and 123 corresponds to the number of elements, this number is stored in the samples header but Sample::payload().len() will return 123 * MessageTypeDetails::payload.size, the bytes used by the sample. When the MessageTypeDetails::payload.size is not overridden, Sample::payload().len() == Header::number_of_elements().

The publish subscribe header contains now the number_of_elements that are contained in the sample. The payload layout was removed. In the C API one can now easily obtain the underlying number of elements by just accessing this number directly from the header - no more payload.len() / sizeof(Payload) stuff.

Pre-Review Checklist for the PR Author

1. Add sensible notes for the reviewer
2. PR title is short, expressive and meaningful
3. Relevant issues are linked in the References section
4. Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
5. Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
6. Commits messages are according to this guideline
   • Commit messages have the issue ID ([#123] Add posix ipc example)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
7. Tests follow the best practice for testing
8. Changelog updated in the unreleased section including API breaking changes
9. Assign PR to reviewer
10. All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Unit tests have been written for new behavior
• Public API is documented
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

Closes #498

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.26%. Comparing base (29c71af) to head (6deb81b).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #499      +/-   ##
==========================================
+ Coverage   79.21%   79.26%   +0.05%     
==========================================
  Files         200      200              
  Lines       23728    23762      +34     
==========================================
+ Hits        18795    18835      +40     
+ Misses       4933     4927       -6     

Files with missing lines	Coverage Δ
iceoryx2/src/port/publisher.rs	85.09% <100.00%> (+0.15%)	⬆️
iceoryx2/src/port/subscriber.rs	93.60% <100.00%> (+0.66%)	⬆️
iceoryx2/src/service/builder/publish_subscribe.rs	90.60% <ø> (-0.94%)	⬇️
iceoryx2/src/service/header/publish_subscribe.rs	100.00% <100.00%> (ø)
.../src/service/static_config/message_type_details.rs	100.00% <100.00%> (ø)

... and 11 files with indirect coverage changes

[elBoberido]

Looks good. Just some minor questions before merging this.

[elBoberido]

I have the feeling there is still not a clear definition what payload means and it is used in different contexts with a different meaning, resulting in confusion at some point.

No need to change it now but we need to have unambiguous terminology, else this will haunt us in regular intervals.

[elfenpiff]

Just out of my head:

• payload == all bytes that belong to the user produced data
• slice can be a form of payload
• a slice consists of 1..N elements, therefore we have in this context an element_size?

[elBoberido]

This is what I mean, the term payload is used in multiple contexts and a comment is necessary to point out what it means. In this case it would make more sense to talk about element.size or item.size. A payload can consist of one or multiple elements/items and all elements make up the payload size.

[orecham]

I vote for "element"

[elfenpiff]

I agree, I just speak about payload.size since this is the member of MessageTypeDetails.

I will refactor it so that it becomes more clear.

[elfenpiff]

I tried refactoring this, but this will be a huge task and there are a lot of files affected. I think it would be best to have a separate session for this.

A payload can consist of one or multiple elements/items and all elements make up the payload size.

This is only true for the sliced API. Otherwise, the element is the payload, and it will lead to confusion when only working with this API when suddenly the there is also an element.size. One may think then it relates to members of a struct or something else.

[elfenpiff]

Btw. we are discussing here a comment that becomes only relevant for an internal API that the user shall never use.
I acknowledge that this is important and we have to refine the code. But making the code reflecting the terms correctly without increasing the code complexity is a challenge here in my opinion.

[orecham]

The discussion is about spending some time now to reduce cognitive load for us and other maintainers later.

Although it is internal API, there is still an audience, so clarity still has its merits. In the absolute worst case, we slow down contributions or even scare away potential contributors due to the amount of inside information required.

Leave it up to you to decide if it is worth doing here.

[orecham]

@elfenpiff Let's adjust this in a follow-up if required.