Skip to content

Commit 28f9f71

Browse files
committed
refactor: Implement CodeRabbit suggestions for GHST telemetry tests
Added fake clock and TX buffer space as recommended by CodeRabbit review: - Implemented fakeMicros static variable with testAdvanceMicros() helper - Changed serialTxBytesFree() to return 64 instead of 0 - Added time advancement between processGhst() calls However, tests still fail with zero values, indicating the GHST scheduler requires additional conditions beyond basic time advancement. Possible causes: - Frame type rotation logic - Telemetry enable flags - Inter-frame timing requirements - Schedule index progression Tests remain DISABLED with updated documentation explaining the current state and what infrastructure has been implemented. Further investigation of GHST frame scheduling system is needed to fully enable these tests. Addresses CodeRabbitAI review #3376416184 MAJOR issue and 7 nitpicks. All 38 test files still pass, 2 tests remain disabled with full documentation.
1 parent 5e3dd58 commit 28f9f71

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

src/test/unit/telemetry_ghst_unittest.cc

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ extern "C" {
9292
extern "C" {
9393
uint8_t *ghstGetTelemetryBuf(void); // Returns pointer to telemetryBuf
9494
uint8_t ghstGetTelemetryBufLen(void); // Returns telemetryBufLen
95+
void testAdvanceMicros(uint32_t delta); // Advance fake time for scheduler testing
9596
}
9697

9798
#include "unittest_macros.h"
@@ -118,11 +119,10 @@ uint16_t mAh Drawn
118119
// The firmware flow: processGhst() → writes to ghstFrame → ghstFinalize() → ghstRxWriteTelemetryData() → telemetryBuf
119120
// We access the firmware's telemetryBuf via accessor functions to validate the transmitted frame content.
120121
//
121-
// NOTE: DISABLED - processGhst() doesn't populate telemetryBuf with updated values on subsequent calls.
122-
// First call with zero values works, but second call after updating test variables still returns zeros.
123-
// Likely issue: processGhst() uses a schedule system and may only send frames at specific intervals,
124-
// or there's a state machine that's not being properly reset between calls.
125-
// Requires deeper investigation of GHST telemetry scheduling and state management.
122+
// NOTE: DISABLED - Further investigation needed. Added fake clock and TX buffer space per CodeRabbit suggestion,
123+
// but values still return 0. The GHST scheduler may require additional conditions beyond time advancement
124+
// (e.g., specific frame type rotation, telemetry enable flags, or inter-frame timing requirements).
125+
// Infrastructure is complete for future enabling once scheduler behavior is fully understood.
126126
TEST(TelemetryGhstTest, DISABLED_TestBattery)
127127
{
128128
uint16_t voltage;
@@ -139,6 +139,7 @@ TEST(TelemetryGhstTest, DISABLED_TestBattery)
139139
testmAhDrawn = 0;
140140

141141
initGhstTelemetry();
142+
testAdvanceMicros(50000); // Advance time to allow scheduler window to elapse
142143
processGhst();
143144

144145
// Get telemetry buffer via accessor
@@ -164,6 +165,7 @@ TEST(TelemetryGhstTest, DISABLED_TestBattery)
164165
testAmperage = 2960; // = 29.60A = 29600mA - amperage is in 0.01A steps
165166
testmAhDrawn = 1234;
166167

168+
testAdvanceMicros(50000); // Advance time for next frame
167169
processGhst();
168170

169171
// Get updated buffer (must call accessor again after processGhst)
@@ -184,7 +186,7 @@ TEST(TelemetryGhstTest, DISABLED_TestBattery)
184186
// Validates that firmware correctly sends per-cell voltage instead of pack voltage
185187
// when telemetryConfig()->report_cell_voltage is enabled.
186188
//
187-
// NOTE: DISABLED - Same issue as TestBattery. Telemetry buffer not being updated.
189+
// NOTE: DISABLED - Same as TestBattery. Requires further scheduler investigation.
188190
TEST(TelemetryGhstTest, DISABLED_TestBatteryCellVoltage)
189191
{
190192
uint16_t voltage;
@@ -205,6 +207,7 @@ TEST(TelemetryGhstTest, DISABLED_TestBatteryCellVoltage)
205207
telemetryConfigMutable()->report_cell_voltage = true;
206208

207209
initGhstTelemetry();
210+
testAdvanceMicros(50000); // Advance time to allow scheduler window to elapse
208211
processGhst();
209212

210213
// Get telemetry buffer via accessor
@@ -243,12 +246,17 @@ gpsSolutionData_t gpsSol;
243246

244247
void beeperConfirmationBeeps(uint8_t beepCount) {UNUSED(beepCount);}
245248

246-
uint32_t micros(void) {return 0;}
249+
// Fake time for scheduler-driven telemetry
250+
static uint32_t fakeMicros = 0;
251+
void testAdvanceMicros(uint32_t delta) { fakeMicros += delta; }
252+
uint32_t micros(void) { return fakeMicros; }
253+
uint32_t microsISR(void) { return fakeMicros; }
247254

248255
bool feature(uint32_t) {return true;}
249256

250257
uint32_t serialRxBytesWaiting(const serialPort_t *) {return 0;}
251-
uint32_t serialTxBytesFree(const serialPort_t *) {return 0;}
258+
// Provide space so ghstRxWriteTelemetryData() can "send"
259+
uint32_t serialTxBytesFree(const serialPort_t *) { return 64; }
252260
uint8_t serialRead(serialPort_t *) {return 0;}
253261
void serialWrite(serialPort_t *, uint8_t) {}
254262
void serialWriteBuf(serialPort_t *, const uint8_t *, int) {}
@@ -310,9 +318,6 @@ bool isAmperageConfigured(void) { return true; }
310318
void setRssi(uint16_t, rssiSource_e){}
311319
rssiSource_e rssiSource;
312320

313-
314-
uint32_t microsISR(void) { return 0; };
315-
316321
bool checkGhstTelemetryState(void) {
317322
return true;
318323
}

0 commit comments

Comments
 (0)