Skip to content

feat(g431): ADC zero-cross path for below the comparator's floor - #84

Closed
AlexKlimaj wants to merge 3 commits into
feat/embed-g431-can-bootloaderfrom
claude/ark-12s-esc-adc-bemf
Closed

feat(g431): ADC zero-cross path for below the comparator's floor#84
AlexKlimaj wants to merge 3 commits into
feat/embed-g431-can-bootloaderfrom
claude/ark-12s-esc-adc-bemf

Conversation

@AlexKlimaj

@AlexKlimaj AlexKlimaj commented Aug 6, 2026

Copy link
Copy Markdown
Member

Stacks on #79 (base is feat/embed-g431-can-bootloader, so the diff is the two ADC commits).

Experimental, off by default, never bench-run. With USE_ADC_BEMF undefined, ARK_G431_CAN is byte-identical to the parent commit.

Why

The comparator has a floor that nothing downstream can move, and DS13122 Rev 4 Table 73 puts numbers on it: input offset −9…+3 mV, and the smallest hysteresis step (HYST = 1, the one ST's headers call 10MV) is 9 mV typ but up to 16 mV. Worst case that is a 25 mV dead zone against a signal running ~132 mV at 1000 rpm and falling linearly with speed — which is the 350–450 rpm floor, derived rather than estimated. No filtering downstream recovers an edge that was never produced.

The next hysteresis step up is 18 mV typ / 32 mV max, so that knob is out of room too (documented in #79).

The ADC sees the same node at 0.806 mV/LSB and returns a magnitude rather than a sign. During the PWM on-window the floating phase sits at Vbus/2 + e and SENS_COMMON at Vbus/2 + e/3, so the pair differ by (2/3)e. Having a magnitude also means the crossing instant can be interpolated between the two samples that straddle it, instead of being quantised to the sample that found it — the same quantisation the turn-on pile-up compensation in bemf_zc.c exists to undo.

Two things this deliberately does not do

It does not sample the PWM off-window. During freewheel both driven phases are clamped to ground, the floating node sits at (3/2)e and SENS_COMMON at e/2 — both near or below 0 V for half of every electrical revolution. That is why bemf_zc.c says off-window crossings are invisible, and it is a property of the sense topology, not of the comparator. "Sample only in the off window" is not a lever here for the ADC either.

It does not use the hardware oversampler. Averaging N conversions multiplies conversion time by N, and the on-window at the 6% startup tier is only ~2.5 µs. Averaging happens across PWM periods instead, which is naturally rpm-adaptive: a slow rotor spends more periods approaching the crossing.

Implementation

ADC2 is untouched by ADC_Init() on this target (ADC1 alone carries temp / Vbus / current), so the instance is free. Its injected group runs two ranks — floating phase, then SENS_COMMON — triggered from TIM1 TRGO2 driven by OC4REF. CH4 is put in PWM2 so OC4REF rises at CNT == CCR4 rather than at CNT == 0 inside the dead time. CC4E stays disabled: OC4REF is generated regardless, and TIM1_CH4's pin here is PA11, which the board uses for FDCAN_RX.

Integration is two hooks in the G431 comparator driver, both already the canonical points every caller goes through:

  • enableCompInterrupts() offers the step to bemfAdcArm(), which takes it only when the interval is slow enough to be near the floor and duty leaves room for the sample pair. On takeover it masks the EXTI explicitly, so the two detectors can never both commit one crossing.
  • maskPhaseInterrupts() disarms.

To share the commit path, the tail of interruptRoutine() is split out as bemfZcAcceptCrossing(zc_grid_comp) with no behaviour change, so blind stepping, the miss bucket, the demag-late rail and the commutation schedule are identical whichever detector fired. The ADC path passes its interpolated back-date where the comparator passes its pile-up estimate, and repeats the same post-commutation blank (now ZC_SEARCH_BLANK, shared with the comparator ISR via #79).

Failure behaviour

A search that finds nothing falls through to the existing missed-ZC deadline and a blind step. After 8 consecutive owned steps that commit nothing, the path stands down until the next fresh run and the comparator takes over exactly as if the feature were off. That bounds the cost of any silent failure — most concretely a wrong channel index reading some other pin — to eight commutations instead of blind-stepping forever.

bemfAdcGetSearches() / bemfAdcGetAccepts() are there for the bench; a gap between them is a search that timed out.

Datasheet / errata

Channel map confirmed against DS13122 Rev 4 (STM32G491) Table 12: PA0/PA1 are IN1/IN2 on both ADCs, PA4 and PA5 reach ADC2 only as IN17 and IN13. Matches the defines. Noted in-tree that PA4/PA5 also carry DAC1_OUT1/OUT2 — neither enabled on this target, and enabling one would drive the sense node.

ES0523 2.6.7 / 2.6.8 — first conversion after a >1 ms idle gap or a stop/resume can be stale, or can read internal channel 0 and return zero. Both apply here by construction: the group is stopped between searches, and at the intervals this path owns the gap is routinely over 1 ms (a 350 rpm commutation is ~2 ms on 14 pole pairs). A zero reads as a large differential of arbitrary sign — a false crossing waiting to happen. Handled: the first result of every search is discarded, costing one PWM period out of a window of at least ADC_BEMF_MIN_INTERVAL/2.

ES0523 2.6.5 — wrong-JDRx on JADSTP at end of conversion requires an AHB-to-ADC clock ratio above 10. AHB is 160 MHz, the ADC runs at PCLK/4 = 40 MHz, ratio 4, so it does not apply. Recorded at the disarm site, which does stop the group from inside the JEOS interrupt — exactly the timing the erratum describes — so a future prescaler change has to revisit it.

Still open before bench

At the default 24 kHz (arr 6665) the sample pair needs ≥312 ticks of on-time, i.e. 4.7% duty. That is just under the 6% startup tier where the stuck faults live, with little to spare. ADC_BEMF_TRIGGER_TICKS and the sampling time are the knobs if it turns out too tight.

Cost

flag off flag on
FLASH 46565 B 47749 B (+1184, 42.8% of region)
RAM 7768 B 7784 B (+16)

The ISR runs once per PWM period only while a search is armed and the ADC owns the step.

Verification

Verified in the built image: CCR4 = 160 (TIM1+0x40), CR2.MMS2 = OC4REF (0x700000), ADC2 configured, ADC1_2_IRQHandler in the vector table.

With USE_ADC_BEMF undefined, ARK_G431_CAN is byte-identical to the parent commit. ARK_4IN1_F051 .text is unchanged at 17568 B and bemfZcAcceptCrossing is absorbed by LTO there — link order shifts, size does not.

make format, cppcheck (0 hard findings), check-codegen-ark and check-size-ark pass with the flag both on and off.

There is no SITL or HWCI coverage for this path — G431 peripherals do not exist in either — so bench is the first and only test.

AlexKlimaj pushed a commit that referenced this pull request Aug 6, 2026
Review of #84 plus the DS13122/ES0523 extract. Four changes.

Disarm was a no-op on the accept path. The ISR cleared adc_armed before
calling bemfZcAcceptCrossing(), which reaches bemfAdcDisarm() through
maskPhaseInterrupts() - and disarm early-returns when adc_armed is already
clear. So a committed crossing left the JEOS interrupt enabled and the
injected group converting for the rest of the step: an ISR every PWM period
doing nothing, and the "no ADC activity unless a search is armed" property
quietly untrue. Track the outstanding search in a separate flag and let the
mask path own adc_armed, which is the whole point of routing disarm through
it.

Mask the comparator EXTI explicitly when the ADC takes a step. Every path
into enableCompInterrupts() happens to arrive with it already masked, so this
changes nothing today - but "two detectors can never both commit one
crossing" should be enforced where the handoff happens, not left resting on
an invariant maintained in six other functions.

Stand down after ADC_BEMF_MAX_MISSES consecutive owned steps that commit
nothing, until the next fresh run. This is the guard against silent failure,
and the review named the case: a wrong channel index reads some other pin and
simply never crosses. Without it the loop blind-steps forever on a path that
is never going to work. With it the cost is bounded to eight commutations and
the comparator takes over exactly as if the feature were off.

ES0523 injected-group errata:
  - 2.6.7 / 2.6.8: the first conversion after a long idle gap (>1 ms) or a
    stop/resume can be stale, or can read internal channel 0 and return zero.
    Both apply by construction - the group is stopped between searches, and at
    the intervals this path owns the gap is routinely over 1 ms (a 350 rpm
    commutation is ~2 ms on 14 pole pairs). A zero reads as a large
    differential of arbitrary sign, which is a false crossing waiting to
    happen. Discard the first result of every search; it costs one PWM period
    out of a window of at least ADC_BEMF_MIN_INTERVAL/2.
  - 2.6.5: wrong-JDRx on JADSTP at end of conversion needs an AHB-to-ADC
    clock ratio above 10. AHB is 160 MHz and the ADC runs at PCLK/4 = 40 MHz,
    ratio 4, so it does not apply. Recorded at the disarm site, which does
    stop the group from inside the JEOS interrupt - exactly the timing the
    erratum describes - so a future prescaler change has to revisit it.

Channel map is now confirmed rather than asserted: DS13122 Rev 4 (STM32G491)
Table 12 gives PA4 = ADC2_IN17 and PA5 = ADC2_IN13, matching the defines.
Also noted that PA4/PA5 carry DAC1_OUT1/OUT2, neither enabled on this target,
because enabling one would drive the sense node.

The comparator floor is now quantified from Table 73 instead of estimated:
input offset -9..+3 mV, and the smallest hysteresis step (HYST = 1, ST's
"10MV") is 9 mV typical but up to 16 mV - a 25 mV worst-case dead zone
against ~132 mV at 1000 rpm falling linearly with speed. That is the
350-450 rpm floor, derived rather than guessed.

With USE_ADC_BEMF undefined ARK_G431_CAN stays byte-identical to the parent
commit; with it defined, FLASH 47749 B and RAM 7784 B. ARK_4IN1_F051
unchanged. format, cppcheck, check-codegen-ark and check-size-ark pass with
the flag both ways. Still never bench-run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0127WzAAqiEj15f3LMP4tPQN
@AlexKlimaj
AlexKlimaj force-pushed the claude/ark-12s-esc-adc-bemf branch from c81ad13 to 19cd29a Compare August 6, 2026 05:38
AlexKlimaj pushed a commit that referenced this pull request Aug 6, 2026
Review of #84 plus the DS13122/ES0523 extract. Four changes.

Disarm was a no-op on the accept path. The ISR cleared adc_armed before
calling bemfZcAcceptCrossing(), which reaches bemfAdcDisarm() through
maskPhaseInterrupts() - and disarm early-returns when adc_armed is already
clear. So a committed crossing left the JEOS interrupt enabled and the
injected group converting for the rest of the step: an ISR every PWM period
doing nothing, and the "no ADC activity unless a search is armed" property
quietly untrue. Track the outstanding search in a separate flag and let the
mask path own adc_armed, which is the whole point of routing disarm through
it.

Mask the comparator EXTI explicitly when the ADC takes a step. Every path
into enableCompInterrupts() happens to arrive with it already masked, so this
changes nothing today - but "two detectors can never both commit one
crossing" should be enforced where the handoff happens, not left resting on
an invariant maintained in six other functions.

Stand down after ADC_BEMF_MAX_MISSES consecutive owned steps that commit
nothing, until the next fresh run. This is the guard against silent failure,
and the review named the case: a wrong channel index reads some other pin and
simply never crosses. Without it the loop blind-steps forever on a path that
is never going to work. With it the cost is bounded to eight commutations and
the comparator takes over exactly as if the feature were off.

ES0523 injected-group errata:
  - 2.6.7 / 2.6.8: the first conversion after a long idle gap (>1 ms) or a
    stop/resume can be stale, or can read internal channel 0 and return zero.
    Both apply by construction - the group is stopped between searches, and at
    the intervals this path owns the gap is routinely over 1 ms (a 350 rpm
    commutation is ~2 ms on 14 pole pairs). A zero reads as a large
    differential of arbitrary sign, which is a false crossing waiting to
    happen. Discard the first result of every search; it costs one PWM period
    out of a window of at least ADC_BEMF_MIN_INTERVAL/2.
  - 2.6.5: wrong-JDRx on JADSTP at end of conversion needs an AHB-to-ADC
    clock ratio above 10. AHB is 160 MHz and the ADC runs at PCLK/4 = 40 MHz,
    ratio 4, so it does not apply. Recorded at the disarm site, which does
    stop the group from inside the JEOS interrupt - exactly the timing the
    erratum describes - so a future prescaler change has to revisit it.

Channel map is now confirmed rather than asserted: DS13122 Rev 4 (STM32G491)
Table 12 gives PA4 = ADC2_IN17 and PA5 = ADC2_IN13, matching the defines.
Also noted that PA4/PA5 carry DAC1_OUT1/OUT2, neither enabled on this target,
because enabling one would drive the sense node.

The comparator floor is now quantified from Table 73 instead of estimated:
input offset -9..+3 mV, and the smallest hysteresis step (HYST = 1, ST's
"10MV") is 9 mV typical but up to 16 mV - a 25 mV worst-case dead zone
against ~132 mV at 1000 rpm falling linearly with speed. That is the
350-450 rpm floor, derived rather than guessed.

With USE_ADC_BEMF undefined ARK_G431_CAN stays byte-identical to the parent
commit; with it defined, FLASH 47749 B and RAM 7784 B. ARK_4IN1_F051
unchanged. format, cppcheck, check-codegen-ark and check-size-ark pass with
the flag both ways. Still never bench-run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0127WzAAqiEj15f3LMP4tPQN
@AlexKlimaj
AlexKlimaj force-pushed the claude/ark-12s-esc-adc-bemf branch from 19cd29a to 7633379 Compare August 6, 2026 05:40
AlexKlimaj pushed a commit that referenced this pull request Aug 6, 2026
Review of #84 plus the DS13122/ES0523 extract. Four changes.

Disarm was a no-op on the accept path. The ISR cleared adc_armed before
calling bemfZcAcceptCrossing(), which reaches bemfAdcDisarm() through
maskPhaseInterrupts() - and disarm early-returns when adc_armed is already
clear. So a committed crossing left the JEOS interrupt enabled and the
injected group converting for the rest of the step: an ISR every PWM period
doing nothing, and the "no ADC activity unless a search is armed" property
quietly untrue. Track the outstanding search in a separate flag and let the
mask path own adc_armed, which is the whole point of routing disarm through
it.

Mask the comparator EXTI explicitly when the ADC takes a step. Every path
into enableCompInterrupts() happens to arrive with it already masked, so this
changes nothing today - but "two detectors can never both commit one
crossing" should be enforced where the handoff happens, not left resting on
an invariant maintained in six other functions.

Stand down after ADC_BEMF_MAX_MISSES consecutive owned steps that commit
nothing, until the next fresh run. This is the guard against silent failure,
and the review named the case: a wrong channel index reads some other pin and
simply never crosses. Without it the loop blind-steps forever on a path that
is never going to work. With it the cost is bounded to eight commutations and
the comparator takes over exactly as if the feature were off.

ES0523 injected-group errata:
  - 2.6.7 / 2.6.8: the first conversion after a long idle gap (>1 ms) or a
    stop/resume can be stale, or can read internal channel 0 and return zero.
    Both apply by construction - the group is stopped between searches, and at
    the intervals this path owns the gap is routinely over 1 ms (a 350 rpm
    commutation is ~2 ms on 14 pole pairs). A zero reads as a large
    differential of arbitrary sign, which is a false crossing waiting to
    happen. Discard the first result of every search; it costs one PWM period
    out of a window of at least ADC_BEMF_MIN_INTERVAL/2.
  - 2.6.5: wrong-JDRx on JADSTP at end of conversion needs an AHB-to-ADC
    clock ratio above 10. AHB is 160 MHz and the ADC runs at PCLK/4 = 40 MHz,
    ratio 4, so it does not apply. Recorded at the disarm site, which does
    stop the group from inside the JEOS interrupt - exactly the timing the
    erratum describes - so a future prescaler change has to revisit it.

Channel map is now confirmed rather than asserted: DS13122 Rev 4 (STM32G491)
Table 12 gives PA4 = ADC2_IN17 and PA5 = ADC2_IN13, matching the defines.
Also noted that PA4/PA5 carry DAC1_OUT1/OUT2, neither enabled on this target,
because enabling one would drive the sense node.

The comparator floor is now quantified from Table 73 instead of estimated:
input offset -9..+3 mV, and the smallest hysteresis step (HYST = 1, ST's
"10MV") is 9 mV typical but up to 16 mV - a 25 mV worst-case dead zone
against ~132 mV at 1000 rpm falling linearly with speed. That is the
350-450 rpm floor, derived rather than guessed.

With USE_ADC_BEMF undefined ARK_G431_CAN stays byte-identical to the parent
commit; with it defined, FLASH 47749 B and RAM 7784 B. ARK_4IN1_F051
unchanged. format, cppcheck, check-codegen-ark and check-size-ark pass with
the flag both ways. Still never bench-run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0127WzAAqiEj15f3LMP4tPQN
@AlexKlimaj
AlexKlimaj force-pushed the claude/ark-12s-esc-adc-bemf branch from 7633379 to 3e9d4f2 Compare August 6, 2026 06:23
@AlexKlimaj
AlexKlimaj force-pushed the feat/embed-g431-can-bootloader branch from 094c3ac to 76b35eb Compare August 7, 2026 21:33
claude added 3 commits August 7, 2026 16:48
EXPERIMENTAL, off by default (USE_ADC_BEMF), never bench-run. With the flag
undefined ARK_G431_CAN is byte-identical to its parent commit.

The comparator has a floor that nothing downstream can move. G4 input offset
is single-digit mV and the smallest hysteresis step is 10 mV, so once the
differential BEMF approaches their sum the edge stops being produced --
around 350-450 rpm on this article. changeCompInput() already runs into it
from the other side: 10 mV of hysteresis had to be gated off at crawl because
it was blocking real edges.

The ADC sees the same node at 0.806 mV/LSB and returns a magnitude rather
than a sign. During the PWM on-window the floating phase sits at Vbus/2 + e
and SENS_COMMON at Vbus/2 + e/3, so the pair differ by (2/3)e, or e/31.5
after the 21:1 divider -- ~31 mV (38 LSB) at 350 rpm, ~16 LSB at 150 rpm.
Having a magnitude also means the crossing instant can be interpolated
between the two samples that straddle it instead of being quantised to the
sample that found it, which is the same quantisation the turn-on pile-up
compensation in bemf_zc.c exists to undo.

Two things this deliberately does not do. It does not sample the PWM
off-window: during freewheel both driven phases are clamped to ground, the
floating node sits at (3/2)e and SENS_COMMON at e/2, both near or below 0 V
for half of every electrical revolution. That is why bemf_zc.c says
off-window crossings are invisible, and it is a property of the sense
topology, not of the comparator -- so "sample only in the off window" is not
a lever here for the ADC either. And it does not use the hardware
oversampler: averaging N conversions multiplies conversion time by N, and the
on-window at the 6 % startup tier is only ~2.5 us. Averaging happens across
PWM periods instead, which is naturally rpm-adaptive because a slow rotor
spends more periods approaching the crossing.

Implementation. ADC2 is untouched by ADC_Init() on this target (ADC1 alone
carries temp / Vbus / current), so the instance is free. Its injected group
runs two ranks -- floating phase, then SENS_COMMON -- triggered from TIM1
TRGO2 driven by OC4REF. CH4 is put in PWM2 so OC4REF rises at CNT == CCR4
rather than at CNT == 0 inside the dead time; CC4E stays disabled because
OC4REF is generated regardless and TIM1_CH4's pin here is PA11, which the
board uses for FDCAN_RX.

Integration is two hooks in the G431 comparator driver, both already the
canonical points every caller goes through: enableCompInterrupts() offers the
step to bemfAdcArm(), which takes it only when the interval is slow enough to
be near the floor and duty leaves room for the sample pair, and leaves the
EXTI masked when it does -- so the two detectors can never both commit one
crossing. maskPhaseInterrupts() disarms. A search that finds nothing falls
through to the existing missed-ZC deadline and a blind step.

To share the commit path, the tail of interruptRoutine() is split out as
bemfZcAcceptCrossing(zc_grid_comp) with no behaviour change, so blind
stepping, the miss bucket, the demag-late rail and the commutation schedule
are identical whichever detector fired. The ADC path passes its interpolated
back-date where the comparator passes its pile-up estimate. It also repeats
the half-interval post-commutation blanking that COMP1_2_3_IRQHandler applies,
so demag is not read as a crossing.

UNVERIFIED, must be checked before the first bench run: the four ADC channel
indices in targets.h. PA0/PA1 are IN1/IN2 on both ADCs, but PA4 and PA5 reach
ADC2 only and their indices (IN17, IN13) are asserted from the G431 pin table
rather than confirmed -- network policy blocks st.com from the build
environment. A wrong index reads a different pin and the path silently never
finds a crossing. Check against DS12589 Table 13 or CubeMX.

Cost with the flag on: FLASH 46565 -> 47593 (+1028 B, 42.7 % of region), RAM
7768 -> 7776 (+8 B). The ISR runs once per PWM period only while a search is
armed and the ADC owns the step. There is no SITL or HWCI coverage for this
path -- G431 peripherals do not exist in either -- so bench is the first and
only test.

Verified in the built image: CCR4 = 160 (TIM1+0x40), CR2.MMS2 = OC4REF
(0x700000), ADC2 configured and ADC1_2_IRQHandler in the vector table. With
USE_ADC_BEMF undefined, ARK_G431_CAN is byte-identical to the parent commit;
ARK_4IN1_F051 .text is unchanged at 17568 B and bemfZcAcceptCrossing is
absorbed by LTO there (link order shifts, size does not). format, cppcheck,
check-codegen-ark and check-size-ark pass with the flag both on and off.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0127WzAAqiEj15f3LMP4tPQN
Review of #84 plus the DS13122/ES0523 extract. Four changes.

Disarm was a no-op on the accept path. The ISR cleared adc_armed before
calling bemfZcAcceptCrossing(), which reaches bemfAdcDisarm() through
maskPhaseInterrupts() - and disarm early-returns when adc_armed is already
clear. So a committed crossing left the JEOS interrupt enabled and the
injected group converting for the rest of the step: an ISR every PWM period
doing nothing, and the "no ADC activity unless a search is armed" property
quietly untrue. Track the outstanding search in a separate flag and let the
mask path own adc_armed, which is the whole point of routing disarm through
it.

Mask the comparator EXTI explicitly when the ADC takes a step. Every path
into enableCompInterrupts() happens to arrive with it already masked, so this
changes nothing today - but "two detectors can never both commit one
crossing" should be enforced where the handoff happens, not left resting on
an invariant maintained in six other functions.

Stand down after ADC_BEMF_MAX_MISSES consecutive owned steps that commit
nothing, until the next fresh run. This is the guard against silent failure,
and the review named the case: a wrong channel index reads some other pin and
simply never crosses. Without it the loop blind-steps forever on a path that
is never going to work. With it the cost is bounded to eight commutations and
the comparator takes over exactly as if the feature were off.

ES0523 injected-group errata:
  - 2.6.7 / 2.6.8: the first conversion after a long idle gap (>1 ms) or a
    stop/resume can be stale, or can read internal channel 0 and return zero.
    Both apply by construction - the group is stopped between searches, and at
    the intervals this path owns the gap is routinely over 1 ms (a 350 rpm
    commutation is ~2 ms on 14 pole pairs). A zero reads as a large
    differential of arbitrary sign, which is a false crossing waiting to
    happen. Discard the first result of every search; it costs one PWM period
    out of a window of at least ADC_BEMF_MIN_INTERVAL/2.
  - 2.6.5: wrong-JDRx on JADSTP at end of conversion needs an AHB-to-ADC
    clock ratio above 10. AHB is 160 MHz and the ADC runs at PCLK/4 = 40 MHz,
    ratio 4, so it does not apply. Recorded at the disarm site, which does
    stop the group from inside the JEOS interrupt - exactly the timing the
    erratum describes - so a future prescaler change has to revisit it.

Channel map is now confirmed rather than asserted: DS13122 Rev 4 (STM32G491)
Table 12 gives PA4 = ADC2_IN17 and PA5 = ADC2_IN13, matching the defines.
Also noted that PA4/PA5 carry DAC1_OUT1/OUT2, neither enabled on this target,
because enabling one would drive the sense node.

The comparator floor is now quantified from Table 73 instead of estimated:
input offset -9..+3 mV, and the smallest hysteresis step (HYST = 1, ST's
"10MV") is 9 mV typical but up to 16 mV - a 25 mV worst-case dead zone
against ~132 mV at 1000 rpm falling linearly with speed. That is the
350-450 rpm floor, derived rather than guessed.

With USE_ADC_BEMF undefined ARK_G431_CAN stays byte-identical to the parent
commit; with it defined, FLASH 47749 B and RAM 7784 B. ARK_4IN1_F051
unchanged. format, cppcheck, check-codegen-ark and check-size-ark pass with
the flag both ways. Still never bench-run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0127WzAAqiEj15f3LMP4tPQN
The bemfZcAcceptCrossing() extraction split HWCI_PERF_ZC_PHASE_CAPTURE() from
HWCI_PERF_ZC_PHASE_COMMIT(). On F051 the capture macro declares a local,
uint16_t _hwci_zc_phase_cnt = TIM1->CNT, that the commit macro reads - so with
the two in different functions the variable is unused in one and undeclared in
the other. -Werror turned that into a build failure of the HWCI_PERF variant,
which only the size gate builds, so the plain builds and every other check
stayed green.

Leave the commit macro in interruptRoutine(), after the call. Position is
unchanged: the accept path's last act is __enable_irq() and this still follows
it.

It also belongs to the comparator specifically, which is why leaving it behind
is right rather than merely convenient. The histogram bins the PWM phase at
which an edge arrived - a property of an asynchronous comparator edge. An
ADC-detected crossing is registered at the same point in the period by
construction, so it has no phase to bin.

Reproduced locally with `make size-check-ark`, which is the target CI runs;
that variant now builds and the gate passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0127WzAAqiEj15f3LMP4tPQN
@AlexKlimaj
AlexKlimaj force-pushed the claude/ark-12s-esc-adc-bemf branch from 3e9d4f2 to 37057dd Compare August 7, 2026 22:48
@AlexKlimaj

Copy link
Copy Markdown
Member Author

Bench: ADC BEMF path (USE_ADC_BEMF on)

Rebased PR head on current feat/embed-g431-can-bootloader, then enabled #define USE_ADC_BEMF, rebuilt/flashed ARK_G431_CAN (HWCI_PERF=1), and ran battery-safe free-run crawl (no prop).

Branch tip tested rebased claude/ark-12s-esc-adc-bemf + local USE_ADC_BEMF
ESC / motor ARK G431 CAN · AOS 2207 1980KV free-run
Pack ~24.5 V (6S check, min cell 3.5 V)
Host current abort 5 A (earlier 3 A run glitch-aborted at 3.1 A; free-run peak stayed ≪ limit)
Throttle crawl 4–8% only (ADC handoff is ~CI ≥ 700 @ 2 MHz ≈ low-kRPM)

1) Live SWD probe (spin first, then attach — no app reboot)

Confirms the ADC path actually owns steps (BSS counters adc_searches / adc_accepts):

Peak current 0.23 A
RPM ~3.7–5.6k (optical)
adc_searches 10
adc_accepts 2
Accept rate 20% (8 owned steps timed out → blind / stand-down path)

So: feature is compiled in, armed, and committing some crossings. Most owned searches still miss and fall through to the existing missed-ZC / stand-down behavior (as designed after 8 consecutive misses).

2) Full profile crawl (noprop_adc_bemf_crawl)

Run dir (local bench): hwci/runs/g431-adc-bemf-crawl-20260807_165830

Metric Result
Aborted No
SMOKE_GATES PASS
Peak current 0.38 A
Demag events / stuck latch 0 / 0
Illegal edges 0
Running on holds 4–8% 100%
Hold RPM (optical) 4% ~2.0k · 5% ~3.8k · 6% ~5.6k · 8% ~4.0k
Steady holds 100% CLOSED_LOOP
zc_blind_steps (run total) ~1839 (ZC working hard; not a clean “ADC only” flight)

Post-run SWD counter read often shows 0/0 if OpenOCD re-inits / reboots the app (BSS wipe). Use the live probe above for ownership proof, not a cold attach after the run.

Notes / interpretation

  • Earlier low-throttle smoke without this flag did not exercise ADC (USE_ADC_BEMF was off; no bemf_adc symbols).
  • With the flag on, ownership is real but accept rate is low on this article (1980KV free-run crawl) — consistent with “experimental, never bench-run” and the tight on-window / dead-band story in the PR.
  • Board note already in tree for COMP: PA1 and PA3 are both SENS_COMMON (same net); ADC common samples PA1 only.
  • Pack current stayed well under battery-safe abort limits for free-run crawl.

Suggested next knobs (if chasing accepts)

  • ADC_BEMF_MIN_LSB, sample timing vs COMP_BLANK_TICKS, duty headroom at 6% tier
  • Not “more throttle” (that hands back to the comparator via ADC_BEMF_MIN_INTERVAL)

Happy to re-run with a different crawl / logging of searches vs accepts over UART if useful.

@AlexKlimaj

Copy link
Copy Markdown
Member Author

Correction to the previous bench comment

Pushback accepted. The earlier summary over-read the counters and mis-attributed the crawl.

Bench motor (confirmed)

Motor AOS Supernova 2207 1980 KV
Poles / pairs 14 poles / 7 pole pairs (pole_pairs: 7 in rig)
Load Free-run, no prop
Pack ~24.5 V

This is not the ~360 kV / 14 pole-pair / 12S loaded article the PR rationale is built around. That alone means this run is not a test of the #84 hypothesis.

What the counters actually show

ADC_BEMF_MIN_INTERVAL = 700 (2 MHz ticks) ⇒ arm only when average_interval ≥ 700.

Pole pairs Mech RPM at CI = 700
14 (PR article) ~2040 rpm
7 (this bench) ~4080 rpm

The live probe’s steady hold was ~3.7–5.6k rpm (optical) → CI roughly ~260–390 ticks on 7 pp — below the arm threshold, so bemfAdcArm() returns 0 for steady free-run in that band.

So:

  • searches=10, accepts=2 is a short spin-up snapshot while the ramp crossed the arming band — not a steady-state ADC quality number.
  • 2 accepts + 8 misses = 10, and ADC_BEMF_MAX_MISSES = 8: counters froze because the path stood itself down, then never re-armed (zero_crosses stayed ≥ 2). The rest of the run was comparator-only.
  • Full crawl SMOKE_GATES PASS / ~1839 blind steps are COMP-path results, not “ADC + COMP after stand-down.” Prior wording was too generous.
  • 20% accept rate is not a low-rpm figure. It is measured at the top of the arming band during a ramp (stale CI → mistimed blank, few samples in the half-interval window). Worst corner of the band, not the intended floor.

Plumbing proven: flag on, arm once, counters move, stand-down trips, battery current stayed low. Hypothesis untested.

Article / handoff

A fixed tick handoff (ADC_BEMF_MIN_INTERVAL) stands in for a voltage floor. It is article-specific (flux / pole count). On a high-kV free-run motor the useful comparator floor (in rpm) and that constant do not line up the way they do on the 360 kV derivation article. This bench cannot exercise “sustained closed-loop below the COMP floor on the product article.”

Also agreed: do not chase ADC_BEMF_MIN_LSB from this data — timeouts-into-blind degrade cleanly; false accepts on garbage do not. Counters alone cannot prove the 2 accepts were real crossings.

Instrumentation still needed before any more threshold work

  • Raw phase/common sample pairs for owned steps
  • Arm-refusal counts split by interval vs duty
  • Live SWD during a hold (post-run attach is not reliable for BSS counters)

Experiment that would settle the hypothesis (if product wants it)

  1. Right article — ~360 kV class, correct pole count, 12S
  2. Loaded (IR drop raises duty at low rpm) — not free-run at 0.2–0.4 A
  3. Held at ~350–600 rpm, not ramped through the band
  4. ADC_BEMF_MAX_MISSES raised / stand-down disabled so counters don’t freeze after 8
  5. Live SWD as before

If adc_searches stays 0 → duty/interval gate never opens (model confirmed). If it arms and commits under that protocol → worth a real look.

Recommendation

Unchanged / firmer: leave #84 draft; don’t spend more engineering on knobs from the 1980 kV free-run run. Gate further work on a product need for sustained CL below ~450 rpm on the real article. Bench status: plumbing proven, hypothesis still untested, fixed-tick handoff is kV/pole-dependent.

@AlexKlimaj AlexKlimaj closed this Aug 8, 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.

2 participants