Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(orb-mcu-util): print new diamond evt version #381

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

fouge
Copy link
Collaborator

@fouge fouge commented Feb 19, 2025

also print 'Diamond_unknown' in case binary doesn't support any new version

@fouge fouge requested a review from TheButlah February 19, 2025 11:19
@fouge fouge force-pushed the fouge/mcu-util-new-versions branch from d750940 to 6b84d83 Compare February 19, 2025 11:20
if self.0.version
<= orb_messages::hardware::OrbVersion::HwVersionPearlEv5 as i32
{
write!(f, "EVT{:?}", self.0.version)
Copy link
Member

@Qbicz Qbicz Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
write!(f, "EVT{:?}", self.0.version)
write!(f, "Pearl EV{:?}", self.0.version)

I'd prefer verbose unless any tools depend on it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would prefer too but indeed, this would break things

also print 'Diamond_unknown' in case binary doesn't support any new version
@fouge fouge force-pushed the fouge/mcu-util-new-versions branch from 6b84d83 to dec8a5e Compare February 19, 2025 12:52
fn from_version_i32(value: i32) -> Self;
}
impl OrbVersionFromInt for orb_messages::hardware::OrbVersion {
fn from_version_i32(value: i32) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn from_version_i32(value: i32) -> Self {
const fn from_version_i32(value: i32) -> Self {

Copy link
Collaborator Author

@fouge fouge Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting... is this const a new thing?
what does it bring?

Copy link
Collaborator

@TheButlah TheButlah Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its like constexpr in c++ - evaluation is done at compile-time. Const functions have been around for a long time (possibly since 1.0?). But they are more useful nowadays, and especially after the inline_const feature was stabilized last year: rust-lang/rust#104087.

const mostly gets used in places where assignment to a const or a static variable is necessary.

@fouge fouge merged commit a372c78 into main Feb 24, 2025
14 checks passed
@fouge fouge deleted the fouge/mcu-util-new-versions branch February 24, 2025 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants