test/test_admin_radio/test_main.cpp (2)

1306-1313: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert total warning count for licensed-immediate test.

Unlike its transactional counterpart (line 1328), this test only checks warningsContaining("Licensed mode activated"), never the total capturedWarnings.size(). It wouldn't catch a regression where an unexpected extra warning (e.g., a spurious channel-name warning) is emitted alongside the licensed notice.

✅ Suggested addition

     sendSetChannel(makeChannel(0, meshtastic_Channel_Role_PRIMARY, "", CUSTOM_KEY, 2));
     TEST_ASSERT_EQUAL_INT(1, warningsContaining("Licensed mode activated"));
+    TEST_ASSERT_EQUAL_INT(1, (int)capturedWarnings.size());
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test_admin_radio/test_main.cpp` around lines 1306 - 1313, The
licensed-immediate test only verifies warningsContaining("Licensed mode
activated"), so it can miss extra warnings emitted by sendSetChannel() or
ensureLicensedOperation(). Update
test_warn_license_noTransaction_emittedImmediately to also assert the total
capturedWarnings.size() matches the expected single warning, mirroring the
transactional test’s stronger check.

---

1170-1223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Trim the multi-line banner comment.

The 8-line section-header comment exceeds the "one or two lines at most" limit for code comments. The rest of the helpers (warningsContaining, makeChannel, sendAdmin, etc.) look correct.

✏️ Suggested trim

-// -----------------------------------------------------------------------
-// Channel-configuration warning + coalescing tests
-//
-// These exercise the real incoming-admin-message path (handleReceivedProtobuf):
-// begin_edit_settings / set_channel / commit_edit_settings. Warnings raised while a
-// transaction is open must be deferred and collapsed into a single notification at
-// commit; outside a transaction each save emits its own single message immediately.
-// -----------------------------------------------------------------------
+// Channel-configuration warning + coalescing tests: exercises the real incoming-admin
+// path (begin/set_channel/commit) where transaction-scoped warnings must collapse to one.

As per coding guidelines, "Keep code comments minimal: one or two lines at most, only explain the why when it is not obvious, and avoid multi-paragraph explanatory comments."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test_admin_radio/test_main.cpp` around lines 1170 - 1223, Trim the
section-header comment above the channel-configuration warning/coalescing test
helpers so it is no more than one or two lines. Keep the intent clear but remove
the long explanatory banner, leaving the helper functions like
warningsContaining, makeChannel, sendAdmin, and sendSetChannel unchanged.

Source: Coding guidelines

src/modules/AdminModule.cpp (1)

1965-1981: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Trim the multi-paragraph doc comments to satisfy the repo comment style.

Both warnOnChannelSet (this block) and warnOnLoraPresetChange (Lines 2089-2106) carry multi-paragraph explanatory comments. The check logic here is already self-explanatory; condense each to a one/two-line summary of the why.

As per coding guidelines for **/*.{cpp,h}: "Keep code comments minimal: one or two lines at most, only explain the why when it is not obvious, and avoid multi-paragraph explanatory comments."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/AdminModule.cpp` around lines 1965 - 1981, Trim the long Doxygen
block above warnOnChannelSet to a one- or two-line comment that only captures
the essential why, and do the same for warnOnLoraPresetChange; remove the
multi-paragraph bullet list and detailed scenario explanations so the comments
match the repo style while keeping the intent clear.

Source: Coding guidelines