Skip to content

[WIP] Add max spin guards to while-loops in test/test_protocol.cpp#5

Closed
Copilot wants to merge 1 commit into
mainfrom
copilot/add-max-spin-guards-to-loops
Closed

[WIP] Add max spin guards to while-loops in test/test_protocol.cpp#5
Copilot wants to merge 1 commit into
mainfrom
copilot/add-max-spin-guards-to-loops

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 21, 2026

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:

  • For every loop in test/test_protocol.cpp of the form while(adapter.available() > 0 && !X) { ... } (where X is any available or ready signal), introduce an integer counter (such as int spins = 0, max_spins = 1000;) and break or fail the test if the max spin count is exceeded.
  • Insert a 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 modify loops that are not waiting for RX/protocol recovery or that do not risk unbounded iteration.
  • Use the same coding conventions as the surrounding code and ensure that inserted messages are unique enough to indicate the nature of the failure in each test.

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:

'TYPE_DATA' is not a member of 'tinylink'

The tests are calling tinylink::TYPE_DATA, but looking at TinyProtocol.h, the message types are defined as a scoped enum class:

enum class MessageType : uint8_t {
    Data = 'D',
    Debug = 'g',
    Req = 'R',
    Done = 'K'
};

So tinylink::TYPE_DATA doesn't exist — the correct reference is tinylink::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 to uint8_t since send() takes uint8_t type):

link.send(static_cast<uint8_t>(tinylink::MessageType::Data), data);

Option B — Add backwards-compat constants to TinyProtocol.h:

static constexpr uint8_t TYPE_DATA  = static_cast<uint8_t>(MessageType::Data);
static constexpr uint8_t TYPE_DEBUG = static_cast<uint8_t>(MessageType::Debug);
static constexpr uint8_t TYPE_REQ   = static_cast<uint8_t>(MessageType::Req);
static constexpr uint8_t TYPE_DONE  = static_cast<uint8_t>(MessageType::Done);

Option B is cleaner — one small addition to TinyProtocol.h fixes 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_DATA in the tests needs to be replaced with static_cast<uint8_t>(tinylink::MessageType::Data). I'll do a targeted sed-style replacement across the whole file now.

Assistant: Done — all tinylink::TYPE_DATA occurrences replaced with static_cast<uint8_t>(tinylink::MessageType::Data) in test/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:

data:
- url: "https://github.com/petesramek/tiny-link/pull/2"
  repository: "petesramek/tiny-link"
  state: "open"
  draft: true
  title: "Reduce payload size limit from 240 → 64 bytes"
  number: 2
  author: "Copilot"
- url: "https://github.com/petesramek/tiny-link/pull/3"
  repository: "petesramek/tiny-link"
  state: "open"
  draft: true
  title: "[WIP] Implement full connection handshake protocol with TinyState enum"
  number: 3
  author: "Copilot"
- url: "https://github.com/petesramek/tiny-link/pull/4"
  repository: "petesramek/tiny-link"
  state: "open"
  draft: true
  title: "Fix AVR build: guard SFINAE adapter validation behind `#ifndef __AVR__`"
  number: 4
  author: "Copilot"

Here's what each PR is doing and how they relate:


PR #2Reduce payload size limit from 240 → 64 bytes

  • Branch: copilot/fix-rawidx-overflow-issuemain
  • What it does: Tightens the static_assert limit to 64 bytes, removes 3 tests that used oversized structs, and adds a CHANGELOG entry. This fixes the latent _rawIdx uint8_t overflow bug.
  • Status: Draft, mergeable_state: blocked (likely requires a review approval).

PR #4Fix AVR build: guard SFINAE validation behind #ifndef __AVR__

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants