fix(mcu): MCU-N5/C4 — runRadarPulseSequence stops shadowing m/n/y globals

runRadarPulseSequence was redeclaring `int m, n, y` at function scope,
which shadowed the file-scope `uint8_t m, n, y` globals at lines
~190-192 that getStatusString reports to the GUI as
BeamPos|Azimuth|ChirpCount. The function's increments updated only the
locals, then discarded them — so telemetry was permanently frozen at
"BeamPos:1|Azimuth:1|ChirpCount:1" no matter how many beam positions
or revolutions had elapsed.

Fix: drop the three local declarations; the body already references
m/n/y by name, so removing the locals lets the writes hit the globals.
A comment documents the pitfall so the locals do not get re-added by
a future cleanup. Numeric ranges are safe (m_max=32, n_max=31,
y_max=50, all fit in uint8_t).

Test: new standalone test_bug16_runradar_shadows_globals.c reproduces
both the buggy (locals shadow globals) and fixed (globals advance)
patterns and asserts the expected post-sweep values
(g_n=16, g_m=1 wraps each iter, g_y=2 after one revolution).

MCU regression: 76/76 (was 75).
This commit is contained in:
Jason
2026-04-27 13:36:28 +05:45
parent e9e301dc50
commit 2c34323bcb
3 changed files with 140 additions and 3 deletions
@@ -65,6 +65,7 @@ TESTS_MOCK_ONLY := test_bug2_ad9523_double_setup \
# Tests that are standalone (no mocks needed, pure logic)
TESTS_STANDALONE := test_bug12_pa_cal_loop_inverted \
test_bug13_dac2_adc_buffer_mismatch \
test_bug16_runradar_shadows_globals \
test_gap3_iwdg_config \
test_gap3_temperature_max \
test_gap3_idq_periodic_reread \
@@ -155,6 +156,9 @@ test_bug12_pa_cal_loop_inverted: test_bug12_pa_cal_loop_inverted.c
test_bug13_dac2_adc_buffer_mismatch: test_bug13_dac2_adc_buffer_mismatch.c
$(CC) $(CFLAGS) $< -lm -o $@
test_bug16_runradar_shadows_globals: test_bug16_runradar_shadows_globals.c
$(CC) $(CFLAGS) $< -o $@
# Gap-3 safety tests -- mock-only (needs spy log for GPIO sequence)
test_gap3_emergency_stop_rails: test_gap3_emergency_stop_rails.c $(MOCK_OBJS)
$(CC) $(CFLAGS) $(INCLUDES) $< $(MOCK_OBJS) -o $@
@@ -0,0 +1,132 @@
/*
* test_bug16_runradar_shadows_globals.c
*
* MCU-N5/C4: runRadarPulseSequence() in main.cpp used to declare local
* `int m, n, y` that shadowed the file-scope globals consumed by
* getStatusString() (BeamPos/Azimuth/ChirpCount). Result: telemetry
* was frozen at "BeamPos:1|Azimuth:1|ChirpCount:1" forever no matter
* how many beam positions or revolutions had elapsed.
*
* This is a host-side static-pattern test. We replay the structural
* loop from runRadarPulseSequence using two sibling helpers one
* with the pre-fix shadowing, one with the post-fix global-only
* pattern and assert that:
* (a) the shadowing version leaves globals at their initial value,
* (b) the fixed version updates the globals exactly as expected.
*
* No HAL, no mocks; just C arithmetic.
*/
#include <assert.h>
#include <stdio.h>
#include <stdint.h>
/* Match main.cpp lines 182-183, 190-193 */
static const int m_max = 32;
static const int n_max = 31;
static uint8_t g_m;
static uint8_t g_n;
static uint8_t g_y;
static uint8_t g_y_max = 50;
static void reset_globals(void)
{
g_m = 1;
g_n = 1;
g_y = 1;
}
/* Pre-fix replica of runRadarPulseSequence body (locals shadow globals). */
static void run_buggy(void)
{
int m = 1;
int n = 1;
int y = 1;
for (int beam_pos = 0; beam_pos < 15; beam_pos++) {
m += m_max / 2;
m += m_max / 2;
m += m_max / 2;
if (m > m_max) m = 1;
n++;
if (n > n_max) n = 1;
}
y++; if (y > g_y_max) y = 1;
/* The locals are discarded; globals stay untouched. */
(void)m; (void)n; (void)y;
}
/* Post-fix: same body, no local redeclaration — references globals. */
static void run_fixed(void)
{
for (int beam_pos = 0; beam_pos < 15; beam_pos++) {
g_m += m_max / 2;
g_m += m_max / 2;
g_m += m_max / 2;
if (g_m > m_max) g_m = 1;
g_n++;
if (g_n > n_max) g_n = 1;
}
g_y++; if (g_y > g_y_max) g_y = 1;
}
int main(void)
{
int failures = 0;
/* (a) Buggy version: globals must remain at 1. */
reset_globals();
run_buggy();
if (g_m != 1 || g_n != 1 || g_y != 1) {
fprintf(stderr,
"FAIL: buggy run modified globals (m=%u n=%u y=%u) — "
"shadowing replica is broken\n", g_m, g_n, g_y);
failures++;
} else {
printf("PASS: pre-fix shadowing leaves globals at (1,1,1)\n");
}
/* (b) Fixed version: 15 beam positions advance n; m wraps; y bumps. */
reset_globals();
run_fixed();
/* After 15 iterations of n++, with n_max=31, n should be 16. */
if (g_n != 16) {
fprintf(stderr, "FAIL: g_n=%u (expected 16)\n", g_n);
failures++;
} else {
printf("PASS: g_n advanced to 16 after 15 beam positions\n");
}
/* m: each iter adds 3*(m_max/2)=48; reset to 1 when m>m_max=32.
* 1+48=49 -> reset to 1. So after every iter m=1. */
if (g_m != 1) {
fprintf(stderr, "FAIL: g_m=%u (expected 1 after wrap)\n", g_m);
failures++;
} else {
printf("PASS: g_m wraps to 1 each iteration as designed\n");
}
/* y bumped exactly once at end of sweep. */
if (g_y != 2) {
fprintf(stderr, "FAIL: g_y=%u (expected 2)\n", g_y);
failures++;
} else {
printf("PASS: g_y advanced from 1 -> 2 after one revolution\n");
}
/* (c) Belt-and-suspenders: a static-string scan of main.cpp asserts
* that the three shadowing declarations are gone. Skipped here
* gated by the runtime checks above plus the project's regression
* grep. The Makefile target keeps this test cheap. */
if (failures > 0) {
fprintf(stderr, "\n*** %d failure(s) ***\n", failures);
return 1;
}
printf("\nAll checks passed.\n");
return 0;
}