mirror of
https://github.com/agessaman/MeshCore.git
synced 2026-08-28 00:44:06 +00:00
test(mqtt): extract + test remaining inline MQTT decision points (Phase 6)
Close the host-testable gaps named in Phase 6 of STABILITY_TESTABILITY_HANDOFF.md
by moving the last inline decision logic into the pure, host-tested policy seams:
- WiFi STA reconnect backoff: extract the inline ladder + wrap-safe timing from
handleWiFiConnection() into MQTTConnectionPolicy::{wifiReconnectBackoffMs,
wifiReconnectDue,nextWifiBackoffAttempt}. Behavior-preserving (elapsedMs is the
wrap-safe form of the old ULONG_MAX branch); ladder/clamp/attempt-cap unchanged.
- Publication outcome pairing: name the (packet, raw) -> delivered contract as
MQTTPacketQueuePolicy::queuedPacketPublished() and wire both queue-drain sites;
partial success = completed, not retried.
- Freeze MQTTPublicationType enum values in a test (the bridge-side MQTTMessageType
alignment is already enforced by a compile-time static_assert).
Adds host tests for all three (exact boundaries + millis() rollover). Native suite
green (13 dirs); non-PSRAM observer firmware smoke build compiles.
WebConfig batch/reboot/stop state-machine extraction and queue-orchestration
coverage remain open (tracked in the Phase 6 status).
This commit is contained in:
@@ -30,7 +30,7 @@ index.
|
||||
| 4 | Ownership and teardown test seams | Seams + ownership doc + teardown tests done; production rewiring deferred to Phase 5 |
|
||||
| 5 | Cooperative MQTT shutdown | Minimal cooperative `end()` + `begin()` guard + OTA barrier implemented on branch `phase5/cooperative-mqtt-shutdown` (native green, firmware smoke build green); NOT hardware-validated. Volatile-handshake replacement + snapshot-consumer repointing deferred |
|
||||
| — | OTA teardown barrier | Implemented — flash gated on a clean MQTT stop in `simple_repeater`; not hardware-validated |
|
||||
| 6 | Request/queue/connection/publication integration tests | Not started |
|
||||
| 6 | Request/queue/connection/publication integration tests | Partial: WiFi-backoff + publish-outcome + enum-alignment gaps extracted and host-tested on branch `phase6/integration-tests`; WebConfig batch/reboot/stop state machine and queue-orchestration coverage still open |
|
||||
| 7 | Uptime, memory, and fault-injection gates | Not started |
|
||||
|
||||
Forward plan, in execution order: **Phase 0 → Phase 4 → Phase 5 (with the OTA
|
||||
@@ -454,7 +454,26 @@ Acceptance criteria:
|
||||
|
||||
### Phase 6: Expand request, queue, connection, and publication integration tests
|
||||
|
||||
**Status: Not started.** Depends on Phase 4/5 lifecycle ownership being stable.
|
||||
**Status: Partial — branch `phase6/integration-tests` (draft PR, base `phase5`).**
|
||||
The remaining *inline* decision points that were host-testable have been extracted
|
||||
into the pure policy seams and covered:
|
||||
|
||||
- WiFi STA reconnect backoff moved out of `handleWiFiConnection()` into
|
||||
`MQTTConnectionPolicy::{wifiReconnectBackoffMs,wifiReconnectDue,
|
||||
nextWifiBackoffAttempt}` (behavior-preserving; adversarially reviewed for
|
||||
rollover/boundary equivalence) with `test_mqtt_connection_policy` cases.
|
||||
- The (packet, raw) publication-outcome pairing named as
|
||||
`MQTTPacketQueuePolicy::queuedPacketPublished()` and wired at both queue-drain
|
||||
sites, with `test_mqtt_packet_queue_policy` cases (partial success = completed).
|
||||
- `MQTTPublicationType` values frozen in `test_mqtt_topic_router`; the
|
||||
bridge-side `MQTTMessageType` alignment was already a compile-time `static_assert`.
|
||||
|
||||
Still open (each a good follow-up PR): the **WebConfig POST/result/reboot/stop
|
||||
state machine** (largest gap — all inline in `WebConfigServer.cpp`; natural to
|
||||
extract a pure `WebConfigBatch` seam mirroring `MQTTLifecycle.h`) and the
|
||||
**queue-orchestration** behaviors (FIFO ordering, evict/requeue-failure interplay,
|
||||
the two adapters' drop-vs-keep-head divergence), which need a fake-queue harness.
|
||||
The original scope list follows.
|
||||
|
||||
After lifecycle ownership is stable, broaden deterministic integration coverage:
|
||||
|
||||
|
||||
@@ -86,6 +86,35 @@ static inline bool circuitBreakerProbeDue(uint32_t now, uint32_t last_attempt) {
|
||||
return elapsedMs(now, last_attempt) >= kCircuitBreakerProbeMs;
|
||||
}
|
||||
|
||||
// WiFi station reconnect backoff. The bridge drives its own STA reconnect loop
|
||||
// separate from the per-slot MQTT reconnects, with a slightly longer first rung
|
||||
// (15 s vs the slot ladder's 10 s). Extracted from handleWiFiConnection() so the
|
||||
// ladder and its wrap-safe timing are exercised by host tests instead of a
|
||||
// second inline copy of the backoff math.
|
||||
static inline uint32_t wifiReconnectBackoffMs(uint8_t attempt) {
|
||||
static const uint32_t kBackoffMs[] = {
|
||||
15000UL, 30000UL, 60000UL, 120000UL, 300000UL
|
||||
};
|
||||
const uint8_t index = attempt < 5 ? attempt : 4;
|
||||
return kBackoffMs[index];
|
||||
}
|
||||
|
||||
// A reconnect is due only once the link has been down for the current rung AND
|
||||
// no attempt has been made within that rung (both measured wrap-safely). This
|
||||
// mirrors the two-part guard the bridge applied inline.
|
||||
static inline bool wifiReconnectDue(uint32_t now, uint32_t disconnected_since,
|
||||
uint32_t last_attempt, uint8_t attempt) {
|
||||
const uint32_t delay = wifiReconnectBackoffMs(attempt);
|
||||
return elapsedMs(now, disconnected_since) >= delay &&
|
||||
elapsedMs(now, last_attempt) >= delay;
|
||||
}
|
||||
|
||||
// The attempt counter climbs to 5 and then saturates; the index clamp in
|
||||
// wifiReconnectBackoffMs() holds it at the 300 s rung.
|
||||
static inline uint8_t nextWifiBackoffAttempt(uint8_t attempt) {
|
||||
return attempt < 5 ? static_cast<uint8_t>(attempt + 1) : attempt;
|
||||
}
|
||||
|
||||
// Each later slot expires up to five percent of the base lifetime earlier,
|
||||
// capped at five minutes per slot. Runtime slot indexes are bounded by the
|
||||
// persisted MQTT slot count; the final clamp also prevents underflow if this
|
||||
|
||||
@@ -87,6 +87,17 @@ struct RetryDecision {
|
||||
uint32_t next_retry_ms;
|
||||
};
|
||||
|
||||
// A queued packet counts as delivered if EITHER its structured-packet publish
|
||||
// or its raw-frame publish reached at least one slot. Partial success (one
|
||||
// succeeds while the other fails or was not attempted) is still success — the
|
||||
// packet completes and is not retried. This is the (packet, raw) outcome pairing
|
||||
// fed to retryDecision(); naming it keeps the "partial publish = done" contract
|
||||
// explicit and host-tested rather than inline in the bridge's queue drain.
|
||||
static inline bool queuedPacketPublished(bool packet_published,
|
||||
bool raw_published) {
|
||||
return packet_published || raw_published;
|
||||
}
|
||||
|
||||
static inline RetryDecision retryDecision(bool any_published,
|
||||
uint8_t retry_attempts,
|
||||
uint32_t now) {
|
||||
|
||||
@@ -2334,18 +2334,17 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) {
|
||||
}
|
||||
}
|
||||
} else if (_wifi_disconnected_time > 0) {
|
||||
unsigned long disconnected_duration = now - _wifi_disconnected_time;
|
||||
static const unsigned long WIFI_BACKOFF_MS[] = { 15000, 30000, 60000, 120000, 300000 };
|
||||
unsigned int idx = (_wifi_reconnect_backoff_attempt < 5) ? _wifi_reconnect_backoff_attempt : 4;
|
||||
unsigned long delay_ms = WIFI_BACKOFF_MS[idx];
|
||||
unsigned long elapsed_since_attempt = (now >= _last_wifi_reconnect_attempt)
|
||||
? (now - _last_wifi_reconnect_attempt)
|
||||
: (ULONG_MAX - _last_wifi_reconnect_attempt + now + 1);
|
||||
if (disconnected_duration >= delay_ms && elapsed_since_attempt >= delay_ms) {
|
||||
// Backoff ladder + wrap-safe timing live in MQTTConnectionPolicy (Phase 6),
|
||||
// exercised by host tests. Behavior is unchanged: both the link-down
|
||||
// duration and the since-last-attempt interval must clear the current rung
|
||||
// (elapsedMs is the wrap-safe form of the old ULONG_MAX branch).
|
||||
if (MQTTConnectionPolicy::wifiReconnectDue(
|
||||
(uint32_t)now, (uint32_t)_wifi_disconnected_time,
|
||||
(uint32_t)_last_wifi_reconnect_attempt,
|
||||
_wifi_reconnect_backoff_attempt)) {
|
||||
_last_wifi_reconnect_attempt = now;
|
||||
if (_wifi_reconnect_backoff_attempt < 5) {
|
||||
_wifi_reconnect_backoff_attempt++;
|
||||
}
|
||||
_wifi_reconnect_backoff_attempt =
|
||||
MQTTConnectionPolicy::nextWifiBackoffAttempt(_wifi_reconnect_backoff_attempt);
|
||||
WiFi.disconnect();
|
||||
WiFi.begin(_obs->wifi_ssid, _obs->wifi_password);
|
||||
}
|
||||
@@ -2664,7 +2663,7 @@ void MQTTBridge::processPacketQueue() {
|
||||
raw_published = publishRaw(&queued.packet_copy);
|
||||
}
|
||||
|
||||
bool any_published = packet_published || raw_published;
|
||||
bool any_published = MQTTPacketQueuePolicy::queuedPacketPublished(packet_published, raw_published);
|
||||
const MQTTPacketQueuePolicy::RetryDecision retry =
|
||||
MQTTPacketQueuePolicy::retryDecision(
|
||||
any_published, queued.retry_attempts,
|
||||
@@ -2790,7 +2789,7 @@ void MQTTBridge::processPacketQueue() {
|
||||
raw_published = publishRaw(&queued.packet_copy);
|
||||
}
|
||||
|
||||
bool any_published = packet_published || raw_published;
|
||||
bool any_published = MQTTPacketQueuePolicy::queuedPacketPublished(packet_published, raw_published);
|
||||
const MQTTPacketQueuePolicy::RetryDecision retry =
|
||||
MQTTPacketQueuePolicy::retryDecision(
|
||||
any_published, queued.retry_attempts,
|
||||
|
||||
@@ -152,6 +152,49 @@ TEST(MQTTConnectionPolicy, JwtClockNeedsNtpOrAReasonableWallClock) {
|
||||
EXPECT_TRUE(Policy::jwtClockAvailable(true, 0U));
|
||||
}
|
||||
|
||||
TEST(MQTTConnectionPolicy, WifiBackoffLadderStartsAtFifteenSecondsAndSaturates) {
|
||||
EXPECT_EQ(15000U, Policy::wifiReconnectBackoffMs(0));
|
||||
EXPECT_EQ(30000U, Policy::wifiReconnectBackoffMs(1));
|
||||
EXPECT_EQ(60000U, Policy::wifiReconnectBackoffMs(2));
|
||||
EXPECT_EQ(120000U, Policy::wifiReconnectBackoffMs(3));
|
||||
EXPECT_EQ(300000U, Policy::wifiReconnectBackoffMs(4));
|
||||
// Clamps at the 300 s rung for the saturated attempt count and beyond.
|
||||
EXPECT_EQ(300000U, Policy::wifiReconnectBackoffMs(5));
|
||||
EXPECT_EQ(300000U, Policy::wifiReconnectBackoffMs(200));
|
||||
}
|
||||
|
||||
TEST(MQTTConnectionPolicy, WifiBackoffAttemptClimbsThenSaturatesAtFive) {
|
||||
uint8_t attempt = 0;
|
||||
for (uint8_t expected = 1; expected <= 5; ++expected) {
|
||||
attempt = Policy::nextWifiBackoffAttempt(attempt);
|
||||
EXPECT_EQ(expected, attempt);
|
||||
}
|
||||
// Saturated: never advances past 5 (index stays clamped at the 300 s rung).
|
||||
EXPECT_EQ(5U, Policy::nextWifiBackoffAttempt(attempt));
|
||||
EXPECT_EQ(5U, Policy::nextWifiBackoffAttempt(5));
|
||||
}
|
||||
|
||||
TEST(MQTTConnectionPolicy, WifiReconnectRequiresBothDownAndSinceAttemptToClearRung) {
|
||||
const uint32_t down_since = 1000U;
|
||||
const uint32_t last_attempt = 1000U;
|
||||
const uint8_t attempt = 0; // 15 s rung
|
||||
// Neither interval has elapsed yet.
|
||||
EXPECT_FALSE(Policy::wifiReconnectDue(1000U + 14999U, down_since, last_attempt, attempt));
|
||||
// Down long enough, but an attempt was made only 5 s ago (since-attempt short).
|
||||
EXPECT_FALSE(Policy::wifiReconnectDue(1000U + 15000U, down_since, 1000U + 10000U, attempt));
|
||||
// Both cleared at the exact boundary: due.
|
||||
EXPECT_TRUE(Policy::wifiReconnectDue(1000U + 15000U, down_since, last_attempt, attempt));
|
||||
}
|
||||
|
||||
TEST(MQTTConnectionPolicy, WifiReconnectDueSurvivesMillisRollover) {
|
||||
const uint32_t down_since = std::numeric_limits<uint32_t>::max() - 100U;
|
||||
const uint32_t last_attempt = down_since;
|
||||
const uint8_t attempt = 0; // 15 s rung
|
||||
const uint32_t now = down_since + 15000U; // wraps past zero
|
||||
EXPECT_TRUE(Policy::wifiReconnectDue(now, down_since, last_attempt, attempt));
|
||||
EXPECT_FALSE(Policy::wifiReconnectDue(down_since + 14999U, down_since, last_attempt, attempt));
|
||||
}
|
||||
|
||||
int main(int argc, char** argv) {
|
||||
::testing::InitGoogleTest(&argc, argv);
|
||||
return RUN_ALL_TESTS();
|
||||
|
||||
@@ -151,6 +151,29 @@ TEST(MQTTPacketQueuePolicy, RetrySchedulingDeadlineMayWrapToZero) {
|
||||
decision.retry_attempts));
|
||||
}
|
||||
|
||||
TEST(MQTTPacketQueuePolicy, PartialPublishCountsAsDeliveredEitherWay) {
|
||||
EXPECT_TRUE(QueuePolicy::queuedPacketPublished(true, true));
|
||||
EXPECT_TRUE(QueuePolicy::queuedPacketPublished(true, false)); // packet ok, raw failed
|
||||
EXPECT_TRUE(QueuePolicy::queuedPacketPublished(false, true)); // raw ok, packet failed
|
||||
EXPECT_FALSE(QueuePolicy::queuedPacketPublished(false, false)); // neither reached a slot
|
||||
}
|
||||
|
||||
TEST(MQTTPacketQueuePolicy, PublishOutcomePairingDrivesRetryDecision) {
|
||||
// packet succeeds / raw fails -> completed, no retry.
|
||||
QueuePolicy::RetryDecision d =
|
||||
QueuePolicy::retryDecision(QueuePolicy::queuedPacketPublished(true, false), 0, 1234U);
|
||||
EXPECT_EQ(QueuePolicy::RetryAction::Complete, d.action);
|
||||
|
||||
// raw succeeds / packet fails -> also completed.
|
||||
d = QueuePolicy::retryDecision(QueuePolicy::queuedPacketPublished(false, true), 0, 1234U);
|
||||
EXPECT_EQ(QueuePolicy::RetryAction::Complete, d.action);
|
||||
|
||||
// both fail on a fresh packet -> scheduled for a bounded retry.
|
||||
d = QueuePolicy::retryDecision(QueuePolicy::queuedPacketPublished(false, false), 0, 1234U);
|
||||
EXPECT_EQ(QueuePolicy::RetryAction::Schedule, d.action);
|
||||
EXPECT_EQ(1U, d.retry_attempts);
|
||||
}
|
||||
|
||||
int main(int argc, char** argv) {
|
||||
::testing::InitGoogleTest(&argc, argv);
|
||||
return RUN_ALL_TESTS();
|
||||
|
||||
@@ -171,6 +171,18 @@ TEST(MQTTTopicRouter, RejectsInvalidStyleTypeSlotAndOutput) {
|
||||
EXPECT_FALSE(mqttTopicSlotIndexValid(0, 0));
|
||||
}
|
||||
|
||||
TEST(MQTTTopicRouter, PublicationTypeEnumValuesAreFrozen) {
|
||||
// The bridge passes MQTTBridge::MQTTMessageType to mqttBuildPublicationTopic
|
||||
// as an int; a compile-time static_assert in the bridge ties the two enums
|
||||
// together. Freeze the router side here so its values can't drift on their own.
|
||||
EXPECT_EQ(0, MQTT_PUBLICATION_STATUS);
|
||||
EXPECT_EQ(1, MQTT_PUBLICATION_PACKETS);
|
||||
EXPECT_EQ(2, MQTT_PUBLICATION_RAW);
|
||||
EXPECT_STREQ("status", mqttPublicationTypeName(MQTT_PUBLICATION_STATUS));
|
||||
EXPECT_STREQ("packets", mqttPublicationTypeName(MQTT_PUBLICATION_PACKETS));
|
||||
EXPECT_STREQ("raw", mqttPublicationTypeName(MQTT_PUBLICATION_RAW));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
int main(int argc, char** argv) {
|
||||
|
||||
Reference in New Issue
Block a user