[WIP] Add max spin guards to while-loops in test/test_protocol.cpp#5
Closed
Copilot wants to merge 1 commit into
Closed
[WIP] Add max spin guards to while-loops in test/test_protocol.cpp#5Copilot wants to merge 1 commit into
Copilot wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.
Original prompt
Add max spin/step guards to all relevant while-loops in test/test_protocol.cpp to prevent infinite loops during test execution. Specifically:
while(adapter.available() > 0 && !X) { ... }(where X is any available or ready signal), introduce an integer counter (such asint spins = 0, max_spins = 1000;) and break or fail the test if the max spin count is exceeded.TEST_ASSERT_TRUE_MESSAGE(..., "Did not recover/resynchronize within max spins");(or similar) immediately after such loops, so that tests fail clearly if the protocol's receive state machine does not converge as expected.Do not alter any logic that is not part of test loop protection. Ensure all tests remain otherwise functionally identical. Commit as part of PR #3 ([WIP] Implement full connection handshake protocol with TinyState enum).
The following is the prior conversation context from the user's chat exploration (may be truncated):
Assistant: Different problem this time, and it's a pre-existing one unrelated to the SFINAE fix. The native test build is failing because:
The tests are calling
tinylink::TYPE_DATA, but looking atTinyProtocol.h, the message types are defined as a scoped enum class:So
tinylink::TYPE_DATAdoesn't exist — the correct reference istinylink::MessageType::Data. The tests are using an old flat-constant naming style that no longer matches the enum class definition.There are two ways to fix this:
Option A — Fix the tests to use
tinylink::MessageType::Data(and cast touint8_tsincesend()takesuint8_t type):Option B — Add backwards-compat constants to
TinyProtocol.h:Option B is cleaner — one small addition to
TinyProtocol.hfixes all 20+ failing test calls without touching any test code. Which would you prefer?User: A. library is not released yet, no need to backward compatibility
Assistant: Got it. The fix is straightforward: every
tinylink::TYPE_DATAin the tests needs to be replaced withstatic_cast<uint8_t>(tinylink::MessageType::Data). I'll do a targeted sed-style replacement across the whole file now.Assistant: Done — all
tinylink::TYPE_DATAoccurrences replaced withstatic_cast<uint8_t>(tinylink::MessageType::Data)intest/test_protocol.cpp. CI should pick this up now and the native test build should go green. Keep an eye on #4.User: @Copilot Accepted Confirmation: Are you sure?
Assistant: Here's the current state of
petesramek/tiny-link:Here's what each PR is doing and how they relate:
PR #2 — Reduce payload size limit from 240 → 64 bytes
copilot/fix-rawidx-overflow-issue→mainstatic_assertlimit to 64 bytes, removes 3 tests that used oversized structs, and adds a CHANGELOG entry. This fixes the latent_rawIdxuint8_t overflow bug.mergeable_state: blocked(likely requires a review approval).PR #4 — Fix AVR build: guard SFINAE validation behind
#ifndef __AVR__copilot/fix-sfinae-validation-for-avr→copilot/fix-rawidx-overflow-issue(i.e., stacked on top of PR Reduce payload size limit from 240 → 64 bytes #2)avr-gccdoesn't have<type_traits>, so the S...This pull request was created from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.