Skip to content

chan_simpleusb, chan_usbradio: nonblocking pttkick pipe.#1051

Open
mkmer wants to merge 2 commits into
masterfrom
pttkick-non-blocking
Open

chan_simpleusb, chan_usbradio: nonblocking pttkick pipe.#1051
mkmer wants to merge 2 commits into
masterfrom
pttkick-non-blocking

Conversation

@mkmer
Copy link
Copy Markdown
Collaborator

@mkmer mkmer commented May 19, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved I/O handling by ensuring pipes are created in non-blocking mode from the start, preventing potential blocking issues during operation.

Review Change Stack

@mkmer mkmer added the enhancement New feature or request label May 19, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52875a73-12d4-4245-99db-56fab1a79cb4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Both USB channel driver implementations update their HID thread initialization to create the PTT kick pipe using pipe2 with the O_NONBLOCK flag instead of the standard pipe() call. Error handling and thread lifecycle remain unchanged.

Changes

PTT Pipe Non-blocking I/O

Layer / File(s) Summary
Non-blocking PTT pipe initialization in HID threads
channels/chan_simpleusb.c, channels/chan_usbradio.c
Both HID threads switch from pipe() to pipe2(..., O_NONBLOCK) for creating the pttkick pipe, ensuring non-blocking file descriptors from creation. Error paths log and exit the thread on failure without change.

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: converting the pttkick pipe to non-blocking mode in both chan_simpleusb and chan_usbradio files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pttkick-non-blocking

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@channels/chan_simpleusb.c`:
- Line 1112: The pttkick pipe is opened non-blocking (pipe2(o->pttkick,
O_NONBLOCK)) but both kickptt() (writer) and the pttkick read handler treat res
<= 0 as fatal; change both to ignore expected non-blocking conditions by
checking errno: in kickptt() when write returns -1, if errno is EAGAIN or
EWOULDBLOCK simply return/skip without logging an error, and in the pttkick read
handler (around the current read at ~line 1161) when read returns -1, if errno
is EAGAIN or EWOULDBLOCK do not treat it as an error (just return/continue);
keep other error/EOF handling unchanged.

In `@channels/chan_usbradio.c`:
- Line 1084: The non-blocking pipe created with pipe2(o->pttkick, O_NONBLOCK)
requires kickptt() to treat EAGAIN/EWOULDBLOCK as benign backpressure rather
than a hard error; update the write() error handling inside kickptt() to check
errno and only log/handle true failures, while silently ignoring or returning
success on errno == EAGAIN || errno == EWOULDBLOCK (and keep existing handling
for other errno values), and ensure the code references o->pttkick and the
kickptt() function when making this change so the non-blocking behavior is
correctly honored.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8580ecba-15ce-4e68-8639-2d8e3cbe5a0e

📥 Commits

Reviewing files that changed from the base of the PR and between ce55d4b and bcfc13b.

📒 Files selected for processing (2)
  • channels/chan_simpleusb.c
  • channels/chan_usbradio.c

Comment thread channels/chan_simpleusb.c
Comment thread channels/chan_usbradio.c
@mkmer
Copy link
Copy Markdown
Collaborator Author

mkmer commented May 19, 2026

We "could" block on the write side if something slows down the read side. Since we are polling, we really should never fail to read in the hidthread().

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants