Skip to content

[WIP] Rename MessageType::Req to MessageType::Cmd and add header#14

Closed
Copilot wants to merge 1 commit into
mainfrom
copilot/rename-messagetype-req-to-cmd
Closed

[WIP] Rename MessageType::Req to MessageType::Cmd and add header#14
Copilot wants to merge 1 commit into
mainfrom
copilot/rename-messagetype-req-to-cmd

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 22, 2026

  • Understand repository structure and current state
  • Create src/protocol/ directory and src/protocol/MessageType.h with Cmd='C' and backward-compatible wire helpers
  • Update src/TinyProtocol.h to be an umbrella header that includes protocol/MessageType.h
  • Update keywords.txt to replace TYPE_REQ with TYPE_CMD
  • Add compatibility test (legacy 'R' wire byte decodes to MessageType::Cmd) in test/test_protocol.cpp
  • Update README.md with migration note and MessageType::Cmd usage
Original prompt

Summary

Rename MessageType::Req to MessageType::Cmd (command), use wire byte 'C' for new outgoing frames, and preserve backward compatibility by accepting legacy 'R' wire byte when parsing. Add a dedicated header src/protocol/MessageType.h with helpers for wire conversion and stringification. Update umbrella header (src/TinyProtocol.h) to include the new header and document the migration. Update tests/examples/docs to prefer Cmd/'C'.

Why

  • The project will primarily send commands rather than requests; "Cmd" is clearer.
  • Preserve compatibility by accepting legacy 'R' frames on input so older devices keep working.

Detailed changes to make

  1. Add new header: src/protocol/MessageType.h
  • Define enum class MessageType : uint8_t with values:
    • Data = 'D'
    • Debug = 'g'
    • Cmd = 'C' // new
    • Done = 'K'
  • Provide inline helpers:
    • bool message_type_from_wire(uint8_t b, MessageType &out) — accepts 'C' and legacy 'R' and maps both to MessageType::Cmd.
    • uint8_t message_type_to_wire(MessageType t) — emits 'C' for Cmd.
    • const char* message_type_to_string(MessageType t).

Include exact header content (as implemented in PR) to ensure consistency.

  1. Keep umbrella header src/TinyProtocol.h
  • Update to include "protocol/MessageType.h" (and other focused headers if present). This keeps backward compatibility for existing includes.
  1. Update code, tests, and examples
  • Replace MessageType::Req with MessageType::Cmd in source files, examples, and tests.
  • Replace new outgoing frames to use 'C' (message_type_to_wire or MessageType::Cmd) instead of literal 'R' where code constructs frames to send.
  • Ensure parsing uses message_type_from_wire so both 'R' and 'C' are accepted. Add or update tests accordingly.
  1. Tests and CI
  • Update unit tests that assert on wire bytes to expect 'C' for new frames.
  • Add/ensure a compatibility test that feeds a legacy 'R' frame and confirms it decodes to MessageType::Cmd.
  • Ensure CI runs the native tests so changes are validated.
  1. Documentation
  • Update README.md and examples to use MessageType::Cmd and wire byte 'C'. Add a migration note that parsers accept legacy 'R' and new outgoing frames use 'C'.

Notes for reviewers

  • This change is backward-compatible for parsing; it changes outgoing encoding to 'C' and renames enum member for clarity.
  • The PR keeps TinyProtocol.h as an umbrella header for compatibility and adds the focused header src/protocol/MessageType.h for new code to include directly.

Files to be changed/added

  • Added: src/protocol/MessageType.h
  • Modified: src/TinyProtocol.h (include new header)
  • Modified: various source files, tests, and examples replacing Req/'R' with Cmd/'C' and using the conversion helpers
  • Modified: README.md (migration note and examples)

Testing

  • Run unit tests locally and in CI (native test runner). Ensure parsing of both 'R' and 'C' works and that sending uses 'C'.

Repository

petesramek/tiny-link

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

Assistant: I'm waiting for your confirmation to proceed with creating the pull request in petesramek/tiny-link — please confirm and I'll open it.

User: @Copilot Accepted Confirmation: Confirm agent session

Assistant: Direct answer

  • I started implementing the fix and tests and queued a PR task for petesramek/tiny-link. You can follow progress using the task link below.

What I did and why

  • I began a focused change set that:
    • Fixes the RX state machine so callback mode no longer stalls (call callback without leaving _hasNew set; keep polling mode behavior intact).
    • Removes a leading BOM from src/TinyProtocol.h.
    • Adds a unit test test/test_callback.cpp that asserts callback-mode receives multiple frames.
    • Updates CI to run the native tests so the new test is executed automatically.
  • This addresses the critical bug where registering onReceive() caused update() to stop processing further frames, and it prevents regressions by adding test coverage and CI.

What’s next (automated steps already queued)

  • Implement the TinyLink update() change and commit.
  • Add the new unit test and CI step, run the test suite in CI.
  • Open a PR titled “Fix callback-mode RX stall; add callback unit test and CI coverage” in petesramek/tiny-link and include a description and changelog entry.
  • I’ll report back with the PR link and a summary of the commits and test run results.

Track the task

data:
    - title: '[WIP] Fix bug in TinyLink engine callback processing'
      repository: petesramek/tiny-link
      url: https://github.com/copilot/tasks/pull/PR_kwDORiTJaM7Mgw-a?session_id=cea9d9ba-2c89-4d0d-9ef2-4de6aa747ca0
      state: queued

If you want any changes to scope (e.g., prefer a different callback API, or prefer I keep the README change out of this PR), tell me now and I’ll adjust while the task is applied.

User: if there is a changethat needs to be done to README, then do it. Also update tests if ...

This pull request was created from Copilot chat.


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

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