Implement From<u32> for FwVersion #28
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the From<u32> trait for the FwVersion struct to enable idiomatic Rust conversion from u32 values to FwVersion instances. The conversion extracts the firmware version components (variant, minor, and major) from a u32 using bit manipulation, and is the inverse of the existing From<FwVersion> for u32 implementation.
Changes:
- Adds
From<u32>trait implementation forFwVersionstruct using bit shifts and masks to extract version components
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl From<u32> for FwVersion { | ||
| fn from(ver: u32) -> Self { | ||
| Self { | ||
| variant: (ver & 0xFF) as u8, | ||
| minor: ((ver >> 8) & 0xFFFF) as u16, | ||
| major: ((ver >> 24) & 0xFF) as u8, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The From implementation duplicates the existing FwVersion::new() method (lines 28-34). Consider deprecating FwVersion::new() in favor of using From::from() consistently throughout the codebase, or remove this From implementation if FwVersion::new() is preferred. Having both can lead to inconsistency in how FwVersion instances are created from u32 values.
There was a problem hiding this comment.
Why do we need this? Can't we just call new()?
There was a problem hiding this comment.
@jerrysxie Was used from ADO side, not need for now, close this PR.
| impl From<u32> for FwVersion { | ||
| fn from(ver: u32) -> Self { | ||
| Self { | ||
| variant: (ver & 0xFF) as u8, | ||
| minor: ((ver >> 8) & 0xFFFF) as u16, | ||
| major: ((ver >> 24) & 0xFF) as u8, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding tests for the From and From trait implementations to verify that conversion between FwVersion and u32 is bidirectional and preserves values correctly. The test module (starting at line 1082) includes tests for other protocol structures but lacks coverage for these conversion traits.
3 participants
No description provided.