Skip to content

[WIP] Fix critical bug in TinyLink engine callback processing#12

Closed
Copilot wants to merge 1 commit into
mainfrom
copilot/fix-tinylink-callback-processing-again
Closed

[WIP] Fix critical bug in TinyLink engine callback processing#12
Copilot wants to merge 1 commit into
mainfrom
copilot/fix-tinylink-callback-processing-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 22, 2026

  • Fix bug in src/TinyLink.h: In update(), don't set _hasNew = true when _onReceive is set (callback mode), only set it in polling mode
  • Remove BOM from src/TinyProtocol.h
  • Add test/test_callback.cpp with a test that feeds two frames to a callback-mode TinyLink and asserts both are received
  • Update test/test_runner.cpp to register the new callback test
  • Add clarification to README.md about callback mode not requiring flush()
Original prompt

Summary

Fix a critical bug that causes the TinyLink engine to stop processing incoming frames when a user registers a callback via TinyLink::onReceive(). Remove a leading BOM in src/TinyProtocol.h. Add a native unit test that verifies callback-mode processes multiple frames (test/test_callback.cpp). Update GitHub Actions CI to build and run native tests so the new test runs automatically. Add a short clarification to README.md explaining callback-mode behavior.

Why

  • Current implementation sets _hasNew = true and then calls the user callback; because update() early-returns when _hasNew is true, the engine stops processing further incoming bytes after the first callback invocation. The README advertises that callback mode requires no flush(); this bug breaks that contract.
  • Adding a unit test that feeds multiple frames and registers an onReceive callback will catch regressions.
  • Removing the BOM in src/TinyProtocol.h prevents compiler warnings and subtle tool issues on some toolchains.
  • Running tests in CI prevents regressions from being merged.

Detailed changes to apply

  1. src/TinyLink.h (modify)
  • In TinyLink::update(), adjust the packet-success branch so that when a valid packet is decoded:
    • Set _currType = rtype; _currSeq = rseq; increment _stats.packets; reset _rawIdx = 0.
    • If _onReceive is non-null, call _onReceive(_data) but do NOT set _hasNew. This ensures the engine continues parsing subsequent frames automatically.
    • If _onReceive is null, set _hasNew = true so polling mode works unchanged.
    • Return after handling the packet.
  • Keep overflow and timeout handling unchanged.

Representative code to apply in that branch (exact semantics to implement):

if (payloadLen == sizeof(T)) {
    _currType = rtype;
    _currSeq = rseq;
    _stats.packets++;
    _rawIdx = 0;

    if (_onReceive) {
        _onReceive(_data);
    } else {
        _hasNew = true;
    }

    return;
} else {
    _stats.crcErrs++;
}
  1. src/TinyProtocol.h (modify)
  • Remove any leading BOM / invisible character and save file as UTF-8 without BOM. No other content changes.
  1. test/test_callback.cpp (add)
  • New native test that:
    • Declares a packed struct CallbackData { uint32_t a; uint16_t b; } attribute((packed));
    • Uses tinylink::packet::pack() and tinylink::codec::cobs_encode() to build two valid frames with different seq values.
    • Feeds concatenated frames into a small InMemoryAdapter implementing the adapter contract (isOpen, available, read, write, millis).
    • Instantiates tinylink::TinyLink<CallbackData, InMemoryAdapter> link(adapter);
    • Registers a C-style callback that increments a static volatile counter.
    • Calls link.update() until the counter reaches 2 or a loop limit.
    • Returns 0 on success and non-zero on failure so the existing test runner / CI can detect test failure.
  • Ensure the test compiles in the existing build (no non-portable dependencies).
  1. .github/workflows/cpp-ci.yml (modify or add)
  • Ensure GitHub Actions builds and runs the native tests on ubuntu-latest. Add a job step to compile tests (g++ -std=c++14 or use existing project build commands) and run the test binaries (including test/test_callback). Fail the job if any test returns non-zero.
  1. README.md (modify)
  • Under "Option A: Callback Style (Recommended)" add a short clarification paragraph:

"Callback style behavior (clarification)

Callback mode is fully automatic: when you register a handler with link.onReceive(...), TinyLink will invoke your handler from inside link.update() as frames arrive and will continue parsing subsequent frames automatically. You do not need to call flush() when using callback mode — flush() is only required for the polling pattern (available()/peek()/flush())."

Testing and verification

  • The added native test fails before the fix and passes after the fix.
  • CI will run the native tests on ubuntu-latest to catch regressions.

Files to be changed/added

  • Modified: src/TinyLink.h
  • Modified: src/TinyProtocol.h (BOM removal)
  • Added: test/test_callback.cpp
  • Modified: .github/workflows/cpp-ci.yml (to run native tests)
  • Modified: README.md (small clarification paragraph)

Notes for reviewers

  • The patch is intentionally minimal and backwards-compatible: polling mode behavior is unchanged.
  • The significant behavior change is only that callback-mode no longer stalls; sequence numbering and telemetry increments remain unchanged.

Repository

petesramek/tiny-link

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Review and rework TinyLink project.

Assistant: I'll start by fetching the repository contents and key files from petesramek/tiny-link so I can review the code, README, tests, and the project's structure. I'll run a GitHub read query now.

Assistant: Direct answer / recommendation

  • I reviewed the repository contents and tests, found one functional bug that...

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.

@petesramek petesramek closed this Mar 22, 2026
Copilot AI requested a review from petesramek March 22, 2026 19:05
Copilot stopped work on behalf of petesramek due to an error March 22, 2026 19:05
@petesramek petesramek deleted the copilot/fix-tinylink-callback-processing-again branch March 22, 2026 19:09
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