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

[BFW-6775] beautify: get_dwarf_nr -> dwarf_index #4471

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bkerler
Copy link
Contributor

@bkerler bkerler commented Feb 10, 2025

Using get_dwarf_nr() - 1 for internal/logging use vs get_dwarf_nr() for Marlin/Buddy usage might cause issues, so cleaning it up might help

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);
Copy link
Contributor

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

Copy link
Contributor Author

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto, always true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree.

@bkerler bkerler force-pushed the dwarf_nr_cleanup branch 4 times, most recently from b9f7334 to 09d5b77 Compare February 10, 2025 09:40
@bkerler bkerler requested a review from CZDanol February 10, 2025 09:41
@@ -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;
Copy link
Contributor

@CZDanol CZDanol Feb 10, 2025

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

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@bkerler bkerler requested a review from CZDanol February 13, 2025 19:22
@CZDanol CZDanol changed the title beautify: get_dwarf_nr for Tool pickup, cleanup code beautify: get_dwarf_nr -> dwarf_index Feb 14, 2025
@CZDanol CZDanol changed the title beautify: get_dwarf_nr -> dwarf_index [BFW-6775] beautify: get_dwarf_nr -> dwarf_index Feb 14, 2025
@CZDanol
Copy link
Contributor

CZDanol commented Feb 14, 2025

Internal ticket: BFW-6775

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.

2 participants