Skip to content

Fix homing settling delay and prevent cycleStartEvent queue drop during simultaneous limit switch triggers - #1739

Draft
MitchBradley with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-homing-final-pulloff-issue
Draft

Fix homing settling delay and prevent cycleStartEvent queue drop during simultaneous limit switch triggers#1739
MitchBradley with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-homing-final-pulloff-issue

Conversation

Copilot AI commented May 27, 2026

Copy link
Copy Markdown
Contributor

Two independent correctness bugs are fixed in the homing subsystem.

Bug 1 — _settling_ms never populated (uint32_tuint32_t&)

homing_move()'s settling_ms parameter was declared uint32_t (passed by value) in all kinematics implementations. Because axesVector()/motorVector() write the per-axis configured settle time back through this parameter, the value-copy meant Homing::_settling_ms remained at its initial value of 0 regardless of configuration. As a result delay_ms(_settling_ms) never actually delayed; the intended settling pause after limit contact and after pulloff completion was silently skipped.

Bug 2 — Event queue too small once settling works

With a correctly non-zero _settling_ms, delay_ms() inside the limit-event handler blocks the main loop for the full settle duration (e.g. 250 ms). During that window the GPIO polling task (running every ~5 ms) continues enqueuing pinActiveEvent and limitEvent items for any bouncing switches. With MAX_N_AXIS active limit pins at a 5 ms rate limit, up to ~100 events can accumulate over 250 ms — more than the previous 50-slot queue capacity. If the queue fills before homing_move() enqueues cycleStartEvent, that event is silently dropped by xQueueSend, wake_up() is never called, and the machine hangs indefinitely in Homing state with no motion.

Note: with the original buggy code (_settling_ms = 0), delay_ms(0) yields almost immediately and queue flooding cannot occur. The queue-size fix is therefore a necessary companion to Bug 1's fix — once settling actually works the queue must be large enough to absorb bounce events accumulated during the settle delay.

Changes

  • FluidNC/src/Kinematics/Kinematics.h, Kinematics.cpp, Cartesian.h, Cartesian.cpp, ParallelDelta.h, ParallelDelta.cpp — change homing_move()'s settling_ms parameter from uint32_t to uint32_t& so that Homing::_settling_ms is correctly written back from axesVector()/motorVector() with the per-axis configured settle time.

  • FluidNC/src/Protocol.cpp — increase event_queue capacity from 50 to 200 items so the queue cannot be exhausted by pin-debounce events accumulated during a settle delay, ensuring cycleStartEvent is never dropped:

// Before
event_queue = xQueueCreate(50, sizeof(EventItem));

// After
event_queue = xQueueCreate(200, sizeof(EventItem));

Each EventItem is 8 bytes; the extra 150 slots add 1.2 KB — negligible on ESP32. The new size covers the worst case of MAX_N_AXIS active limit pins × (settle_ms / debounce_ms) bounce events plus headroom.

Copilot AI linked an issue May 27, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix issue with homing not completing final pulloff on XY axes Fix homing pulloff never starting when multi-axis limit switches trigger simultaneously May 27, 2026
Copilot AI requested a review from MitchBradley May 27, 2026 20:00
@MitchBradley

Copy link
Copy Markdown
Collaborator

@copilot I have a concern about what you said, namely "both pinActiveEvents are processed before either limitEvent, so limitEvent(X) sees both axes already in limited()". The problem is that pinActiveEvents do not directly affect the _posLimits and _negLimits variables; they are set by LimitPin::trigger(), which means that the limit events must have been processed.

Copilot AI commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot I have a concern about what you said, namely "both pinActiveEvents are processed before either limitEvent, so limitEvent(X) sees both axes already in limited()". The problem is that pinActiv...

You're right to flag that — the description was imprecise. Here is the accurate chain for a GPIO limit pin:

  1. poll_gpios() (polling task) detects the state change → calls gpio_send_event() → enqueues pinActiveEvent(limitPin) and updates gpios_current
  2. Main loop dequeues pinActiveEvent(limitPin)protocol_do_pin_active → calls limitPin->trigger(true)this is what sets *_posLimits / *_negLimits and then calls EventPin::trigger() which enqueues limitEvent(limitPin)
  3. Main loop dequeues limitEvent(limitPin)protocol_do_limitHoming::limitReached()

So the masks are set inside step 2, not by pinActiveEvent directly. The race occurs because poll_gpios() processes all changed GPIOs in a single pass: when both X and Y switch within the same poll window it enqueues pinActiveEvent(X) then pinActiveEvent(Y) before either limitEvent exists. After the main loop runs through step 2 for both, the queue is [limitEvent(X), limitEvent(Y)]. When limitEvent(X) is dequeued, Y's mask is already set (because step 2 for Y ran first), so limitReached() sees _phaseAxes == 0, calls delay_ms(_settling_ms) inside the handler, and blocks the main loop for 250 ms.

During that 250 ms the polling task continues running. Because gpios_current is updated when each event is sent, a pin that stays steadily active won't re-trigger — but a bouncing switch will generate one new pinActiveEvent per 5 ms rate-limit window, each of which then produces a limitEvent when eventually processed. With two bouncing pins over 250 ms that is up to ~100 events, easily saturating the 50-slot queue before homing_move() can enqueue cycleStartEvent.

@MitchBradley

Copy link
Copy Markdown
Collaborator

@copilot The situation that you have identified might indeed be a problem, but it does not explain the current issue for the following reason. Kinematics.h has a bug whereby homing_move()'s settling_ms parameter is "uint32_t" instead of "uint32_t&". As a result, _settling_ms in Homing.cpp remains at its initial value of 0, and thus delay_ms(_settling_ms) does not delay for long enough to allow the event queue to fill up.

Copilot AI commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot The situation that you have identified might indeed be a problem, but it does not explain the current issue for the following reason. Kinematics.h has a bug whereby homing_move()'s settling_...

You're right — with _settling_ms stuck at 0, delay_ms(0) yields almost immediately so the queue can't fill during settling, which invalidates my queue-overflow explanation for the current code.

The settling_ms type bug is now fixed in commit latest: all homing_move() declarations and implementations have been updated from uint32_t settling_ms to uint32_t& settling_ms (Kinematics.h, Kinematics.cpp, Cartesian.h, Cartesian.cpp, ParallelDelta.h, ParallelDelta.cpp). With this fix Homing::_settling_ms is correctly populated from the axis config values.

With a properly non-zero _settling_ms, the delay_ms() inside the limit-event handler genuinely blocks the event loop for the settle duration, during which bounce events can exhaust the 50-slot queue. So both fixes are needed together: the uint32_t& fix to make settling actually work, and the queue-size increase to prevent cycleStartEvent from being dropped while the event loop is stalled.

@MitchBradley

Copy link
Copy Markdown
Collaborator

@copilot I think that you misunderstood me. My point is that, due to the _settle_ms reference bug, queue flooding cannot happen, thus the original issue remains unexplained. Fixing the reference bug and the queue flooding is probably worthwhile for other reasons, but probably does not solve the problem.

Copilot AI commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot I think that you misunderstood me. My point is that, due to the _settle_ms reference bug, queue flooding cannot happen, thus the original issue remains unexplained. Fixing the reference bug...

You're correct. With _settling_ms = 0 in the original code, delay_ms(0) yields almost immediately — there's no blocking interval during which bounce events could accumulate to overflow the 50-slot queue. Queue flooding is therefore not a valid explanation for the original hang.

The two fixes are still correct on their own merits:

  • The uint32_t& change fixes a real defect: _settling_ms was never populated from the axis configuration, so settling was disabled regardless of what was configured.
  • The queue size increase guards against a new potential hang that would be introduced once settling actually works: with a genuine 250 ms blocking delay inside the event handler, bounce events from two active pins at the 5 ms rate limit can accumulate ~100 items and overflow the 50-slot queue before cycleStartEvent can be enqueued.

But you're right that neither fix addresses the original reported hang.

I've traced the event chain carefully and haven't found a convincing code path to an indefinite hang with _settling_ms = 0. Do you have a hypothesis for what actually causes the original issue? That would help ensure this PR includes the right fix.

Copilot AI changed the title Fix homing pulloff never starting when multi-axis limit switches trigger simultaneously Fix homing settling delay and prevent cycleStartEvent queue drop during simultaneous limit switch triggers May 28, 2026
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.

Problem: Homing sometimes doesn't do the final pulloff on XY axes

2 participants