Audit follow-up: items left out of #570/#571 (hardware verification, semantics decisions, performance and refactors) #572

Context

While going through the library sources against the documentation I collected a number of
findings. The ones that are unambiguous and low-risk to change are in two PRs:

• Fix typos and stale documentation comments (comments/docs only) #570 - typos and stale documentation comments (comments/docs only, no behaviour change)
• Fix confirmed logic bugs found while auditing the library sources (13 fixes, one per commit) #571 - 13 confirmed logic bugs, one commit each

The items below were deliberately left out of those PRs because they either need hardware
verification, change documented semantics, or are proposals rather than fixes. Listing them here
so they don't get lost - happy to turn any of them into a PR if you agree with the direction.

Everything below is from static reading/modelling; I have not measured any of it on hardware.

1. Needs hardware verification: _micros() on AVR (src/common/time_utils.cpp:25)

if((TCCR0B & 0b00000111) == 0x01) return (micros()/32);
else return (micros());

With TCCR0B prescaler 1 the chip runs Timer0 at F_CPU (16 MHz on the Uno), while
micros() assumes the standard prescaler 64 (1 MHz) - its overflow counter adds 1024 us per
overflow instead of 16 us, i.e. it is 64x too large, not 32x. If that reading is right the
workaround should be /64. It obviously works for the users who run with a modified prescaler
(there is a reason this code exists), so this one needs an oscilloscope rather than a reader:
measure _micros()/micros() with the prescaler left at 1 and see which divisor matches. I did
not want to change timing code that many setups depend on without that measurement.

2. Needs a semantic decision: move() downsampling is off by one (FOCMotor.cpp:679)

// monitor() - runs every motion_downsample calls
if( !monitor_downsample || monitor_cnt++ < (monitor_downsample-1) ) return;
// move() - runs every motion_downsample+1 calls
if(motion_cnt++ < motion_downsample) return;

monitor_downsample = 10 means "every 10th loop", motion_downsample = 10 means "every 11th
loop". The documented parameter is shared by both (foc_control.pdf describes downsampling as
running the control loop every N loops), so this is a discrepancy rather than an obvious bug -
but changing it alters the effective control period for every user who sets
motion_downsampling/motion_downsample.

3. Silicon Labs EFR32: dead time silently collapses (efr32_pwm.cpp:300, efr32_mcu.cpp:285)

unsigned int dtiTime = (CMU_ClockFreqGet(timerClock) / 1e3f) * config->deadTimeNs / 1e6f;
if (dtiTime > 64) dtiTime = SILABBS_DEFAULT_DEAD_TIME;   // <- 3 ticks, i.e. tens of ns

uint32_t deadTimeNs = (float)(1e9f / pwm_frequency) * dead_zone;
deadTimeConfig.deadTimeNs = deadTimeNs >> 1;

• When the requested dead time does not fit in the register, it is replaced by 3 timer ticks
  (~40 ns at 76.8 MHz) instead of being clamped to the maximum, i.e. a requested dead time
  becomes an order of magnitude too small - the opposite of the safe direction (shoot-through).
• deadTimeNs >> 1 halves the dead time for the complementary outputs; it is not obvious whether
  that is compensating for something else in the driver, so both need a scope check together.
• The macro is misspelled SILABBS_DEFAULT_DEAD_TIME while the rest of the same files use
  SILABS_* (efr32_pwm.h:8-9 vs efr32_mcu.h:25, SILABS_DEFAULT_DEAD_ZONE). Since it is a
  public-by-accident override hook, renaming it is a (small) API break, hence not in the PRs.

4. characteriseMotor() applies correction_factor inconsistently (FOCMotor.cpp:142,208)

float resistance = voltage / (correction_factor * (r_currents.d - zerocurrent.d));   // R / cf
...
inductanced += fabsf(-(resistance * dt) / log(...)) / correction_factor;             // L / cf

The resistance estimate is already scaled by 1/cf, and the inductance expression is linear in
the resistance, so the inductance ends up scaled by 1/cf^2 while the resistance is scaled by
1/cf. Either the inductance line should not divide again, or the correction factor means
something different from what the comment suggests. I did not want to guess which one is intended

• it changes the numbers reported by motor.characteriseMotor() and the R/L used for tuning.

5. Reported, needs a cheap fix design: switching away from trapezoid leaves a phase off

BLDCMotor::setPhaseVoltage() calls driver->setPhaseState(..., PHASE_OFF, ...) in the trapezoid
branches, but the SinePWM/SpaceVectorPWM branch never re-enables the phases. A 6-PWM driver
stores the phase state and re-applies it on every write, so after a runtime switch from trapezoid
to sine the motor keeps running on two phases (or not at all) until disable()/enable().

The straightforward fix - calling setPhaseState(PHASE_ON, PHASE_ON, PHASE_ON) in the sine branch -
adds a call (and up to three digitalWrites for drivers with enable pins) in the PWM hot path,
so it wants a cheap change detector (e.g. remembering the last foc_modulation used). Left out of
#571 because that adds a member to BLDCMotor and I would rather have your opinion on the
approach first.

6. Smaller inconsistencies, all safe to change but not urgent

7. Performance proposals (measured estimates, not micro-benchmarks)

From the same audit, ranked by (impact x confidence). All numbers are static estimates; the
measurement recipe is motor.loopfoc_time_us / motor.move_time_us before/after.

1. Calibration is dominated by fixed delays: 1.7 s of _delay() per initFOC() plus 100x
   sample averaging just for the current-sense alignment (CurrentSense.cpp, readAverageCurrents()).
   Cutting the ramps/_delay(500)s proportionally would remove roughly 1.5 s from every initFOC().
2. MagneticSensorSPI::read() has an unconditional delayMicroseconds(50) on ESP32 with a
   comment from the author saying it is not needed; removing it removes 50 us per control loop on
   ESP32 (at 1 kHz that is 5 % of the budget, and it is inside the getSensorAngle() path).
3. _sincos() does two table lookups where one would do (the table is already in cache) - two
   divisions/roundings per call saved, called twice per setPhaseVoltage().
4. _normalizeAngle() is called several times per loop and could have a fast path for the
   common in-range case; open-loop increments could wrap directly.
5. PID/LPF: recomputing Tf/(Tf+Ts) and friends every call, plus a division in the filter;
   a constant-Ts fast path would remove divisions from the hot path.
6. AVR: move the 65-entry sine table to flash (PROGMEM) - 130 bytes of the Uno's 2 KB SRAM.
7. Commander table/buffer sizes are hard-coded (20 callbacks, MAX_COMMAND_LENGTH chars);
   making them configurable costs nothing at runtime and saves RAM on small targets.

8. Refactors (no behaviour change, larger diff)

8. De-duplicate the three motor classes (BLDCMotor, StepperMotor, HybridStepperMotor
   share a lot of the cascade/PID/limits/characterisation code).
9. Commander print/parse refactor - the print/parse switch statements are ~400 lines of
   duplicated format strings.
10. Host-side unit tests for the pure logic (_sin, _atan2, PID, LPF, _normalizeAngle,
    the current-sense alignment arithmetic with a fake ADC). The alignment bugs fixed in Fix confirmed logic bugs found while auditing the library sources (13 fixes, one per commit) #571 are
    exactly the class of thing a small host test would catch: they need no hardware, only a
    simulated set of currents.
11. Docs QA in CI - link/anchor/API-name checks exist as throwaway scripts in my workspace;
    they could live in simplefoc.github.io as a GitHub Action so broken links and renamed API
    calls are caught on PRs.

Happy to split any of 1-6 into reviewable PRs, and to contribute 8-11 in stages, if that is
useful - just say which ones you want and in which order.