-
Notifications
You must be signed in to change notification settings - Fork 247
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
[BFW-6775] beautify: get_dwarf_nr -> dwarf_index #4471
base: master
Are you sure you want to change the base?
Conversation
assert(dwarf.get_dwarf_nr() > 0); | ||
const PrusaToolInfo &info = tool_info[dwarf.get_dwarf_nr() - 1]; | ||
assert(dwarf.get_dwarf_nr() < tool_info.size()); | ||
assert(dwarf.get_dwarf_nr() >= 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be always true, dwarf_nr is uint8_t
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
assert(dwarf.get_dwarf_nr() <= tool_info.size()); | ||
assert(dwarf.get_dwarf_nr() > 0); | ||
assert(dwarf.get_dwarf_nr() < tool_info.size()); | ||
assert(dwarf.get_dwarf_nr() >= 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto, always true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
b9f7334
to
09d5b77
Compare
@@ -295,7 +295,7 @@ void ToolsMappingBody::windowEvent([[maybe_unused]] window_t *sender, GUI_event_ | |||
uint16_t get_heatbreak_fan_pwr(); | |||
|
|||
inline uint8_t get_dwarf_nr() const { | |||
return dwarf_nr; | |||
return dwarf_nr - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disrepance between the public getter and private member is even worse than the current state. If we wanna do this, we will need to go even further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could rename get_dwarf_nr
to dwarf_index
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
09d5b77
to
17784c8
Compare
Internal ticket: BFW-6775 |
Using
get_dwarf_nr() - 1
for internal/logging use vsget_dwarf_nr()
for Marlin/Buddy usage might cause issues, so cleaning it up might help