Clean up decoding of known-len component of messages between bootstrap client/server #3881
Conversation
Ben-PH
commented
Apr 28, 2023
•
Ben-PH
commented
- document all added functions
- try in sandbox /simulation/labnet
- unit tests on the added/changed features
- make tests compile
- make tests pass
- add logs allowing easy debugging in case the changes caused problems
- if the API has changed, update the API specification
There was a problem hiding this comment.
I've taken this approach instead of impl Deserializer<ServerMessageLeader> for ServerMessageLeaderDeserializer, as when I tried that, the type-system expectations compromises the error handling/information process without any meaningful benefit.
I've added a point to the follow up issues this relates to regarding the constants.
* Derive the byte-count in a message length in const-context * Use compile-time knowledge
| self.read_exact_timeout(&mut msg_bytes, deadline) | ||
| .map_err(|(err, _consumed)| err)?; |
There was a problem hiding this comment.
piggy backing off this PR: This change should have been part of #3885
| fn from_be_bytes_min(buffer: &[u8], max_value: Self) -> Result<(Self, usize), ModelsError> { | ||
| let read_bytes = Self::be_bytes_min_length(max_value); | ||
| let skip_bytes = Self::MIN_BE_INT_BASE_SIZE - read_bytes; | ||
| let read_bytes = u32_be_bytes_min_length(max_value); |
There was a problem hiding this comment.
const methods as part of traits is... difficult/impossible. Tried a macro, but to make it work "elegantly" would be a poor use of time. Given it's just two types, and not something that is likely to be generalised in the (near) future (from what I can tell), putting in a todo/follow-up is likely to be a distraction, though happy to be corrected.
3948: Hotfix/peek loop r=AurelienFT a=Ben-PH Adapted from #3881 * [ ] document all added functions * [ ] try in sandbox /simulation/labnet * [ ] unit tests on the added/changed features * [ ] make tests compile * [ ] make tests pass * [ ] add logs allowing easy debugging in case the changes caused problems * [ ] if the API has changed, update the API specification Co-authored-by: Ben PHL <benphawke@gmail.com> Co-authored-by: Ben <benphawke@gmail.com> Co-authored-by: AurelienFT <aurelien.foucault@epitech.eu>
