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

idler/selector: simplify init #343

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

Conversation

gudnimg
Copy link
Collaborator

@gudnimg gudnimg commented Dec 25, 2024

The homing is always invalidated in all cases. Let's simply invalidate the homing only in the init method. The state machine's Ready state then decides when it is OK to perform the homing move.

We shouldn't need to guess the axis's position, its safer (and simpler) to assume its always incorrect. Currently the init methods are only called during the boot up sequence.

This change passes the unit tests

Change in memory:
Flash: -126 bytes
SRAM: 0 bytes

The homing is always invalidated in all cases. Let's simply invalidate
the homing only in the init method. The state machine's Ready state then
decides when it is OK to perform the homing move.

We shouldn't need to guess the axis's position, and should assume its not correct. Currently the init methods are only called during the boot up sequence.

Change in memory:
Flash: -126 bytes
SRAM: 0 bytes
Copy link

All values in bytes. Δ Delta to base

ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
-126 0 28006 1667 666 893

Copy link

Automated Test Code Coverage Report

View details...

File Lines Exec Cover
src/application.cpp 169 14 8%
src/application.h 3 3 100%
src/hal/circular_buffer.h 52 52 100%
src/hal/eeprom.h 1 0 0%
src/hal/gpio.h 18 7 38%
src/hal/progmem.h 6 6 100%
src/hal/tmc2130.cpp 113 9 7%
src/hal/tmc2130.h 31 27 87%
src/logic/command_base.cpp 139 118 84%
src/logic/command_base.h 4 3 75%
src/logic/cut_filament.cpp 117 80 68%
src/logic/eject_filament.cpp 77 60 77%
src/logic/feed_to_bondtech.cpp 60 57 95%
src/logic/feed_to_bondtech.h 1 1 100%
src/logic/feed_to_finda.cpp 48 45 93%
src/logic/feed_to_finda.h 1 1 100%
src/logic/home.cpp 18 12 66%
src/logic/load_filament.cpp 85 77 90%
src/logic/load_filament.h 1 0 0%
src/logic/move_selector.cpp 21 0 0%
src/logic/no_command.h 2 1 50%
src/logic/retract_from_finda.cpp 27 21 77%
src/logic/retract_from_finda.h 1 1 100%
src/logic/set_mode.cpp 5 0 0%
src/logic/set_mode.h 1 0 0%
src/logic/start_up.cpp 38 26 68%
src/logic/start_up.h 4 4 100%
src/logic/tool_change.cpp 108 82 75%
src/logic/unload_filament.cpp 76 69 90%
src/logic/unload_to_finda.cpp 40 39 97%
src/logic/unload_to_finda.h 1 1 100%
src/modules/axisunit.h 21 21 100%
src/modules/buttons.cpp 11 11 100%
src/modules/buttons.h 7 7 100%
src/modules/crc.h 13 13 100%
src/modules/debouncer.cpp 28 24 85%
src/modules/debouncer.h 7 7 100%
src/modules/finda.cpp 7 3 42%
src/modules/finda.h 2 2 100%
src/modules/fsensor.cpp 6 6 100%
src/modules/fsensor.h 3 3 100%
src/modules/globals.cpp 47 42 89%
src/modules/globals.h 34 24 70%
src/modules/idler.cpp 86 82 95%
src/modules/idler.h 12 12 100%
src/modules/leds.cpp 44 42 95%
src/modules/leds.h 16 15 93%
src/modules/math.h 6 6 100%
src/modules/motion.cpp 59 40 67%
src/modules/motion.h 66 64 96%
src/modules/movable_base.cpp 73 70 95%
src/modules/movable_base.h 16 16 100%
src/modules/permanent_storage.cpp 144 89 61%
src/modules/protocol.cpp 216 184 85%
src/modules/protocol.h 72 70 97%
src/modules/pulley.cpp 33 25 75%
src/modules/pulley.h 8 5 62%
src/modules/pulse_gen.cpp 95 89 93%
src/modules/pulse_gen.h 53 51 96%
src/modules/selector.cpp 66 62 93%
src/modules/selector.h 5 5 100%
src/modules/speed_table.h 26 24 92%
src/modules/user_input.cpp 39 39 100%
src/modules/user_input.h 12 12 100%
src/modules/voltage.cpp 4 0 0%
src/registers.cpp 104 37 35%
src/unit.h 12 12 100%
TOTAL 2721 2030 74%

TOTAL: 2721 lines of code, 2030 lines executed, 74% covered.

Copy link
Collaborator

@leptun leptun left a comment

Choose a reason for hiding this comment

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

I am a bit suspicious of the selector changes. Need to check if the mmu can unload from the scenario of reset with filament loaded. Previously the axis position was invalidated, but the position itself still matched the loaded slot. Now it no longer matches and I wonder if the logic properly handles that

Copy link
Collaborator

@DRracer DRracer left a comment

Choose a reason for hiding this comment

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

Please be very careful in this case - invalidation of positions has been a subject to many internal discussions. I'm afraid your proposal changes some edge cases which are probably not covered with the unit tests

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