Skip to content

Add controller API wait_for_all_commands_cancelled() - #1555

Open
Gin890 wants to merge 1 commit into
mainfrom
wait-all-cancel
Open

Gin890 wants to merge 1 commit into
mainfrom
wait-all-cancel

Conversation

@Gin890

@Gin890 Gin890 commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

cancel_all_commands() only starts the cancellation: it returns as soon as the cancel request is handed to the connection, and the host marks its command queue empty at that moment, so waiting for the queue proves nothing.

AbstractController::wait_for_all_commands_cancelled() cancels, queues a 10 ms neutral no-op, and waits (bounded by a timeout) for the device to report that the no-op finished. This ensures the controller is usable after this function returns.

We didn't have needs to block waiting for all commands cancelled before as our program is executed in serial and users can watch when the execution finishes on video capture screen.

But for an AI agent it needs the accurate synchronization of the command and the result in-game to reduce its chance of making a mistake.

…in PybindSwitchProController

cancel_all_commands() only starts the cancellation: it returns as soon as the
cancel request is handed to the connection, and the host marks its command
queue empty at that moment, so waiting for the queue proves nothing.

AbstractController::wait_for_all_commands_cancelled() cancels, queues a 10 ms
neutral no-op, and waits (bounded by a timeout) for the device to report that
the no-op finished. Commands are delivered in order, so that report means the
device has processed the cancel and is holding the neutral state. It is a
non-virtual member built only on AbstractController's virtual interface, so it
works for any controller.

PybindSwitchProController::wait_for_all_commands_cancelled() exposes it with a
millisecond timeout. Returns false on timeout, e.g. when the Switch is asleep
and the device can't execute commands. Safe to call from another thread while
one is blocked in wait_for_all_requests().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Gin890
Gin890 marked this pull request as ready for review October 11, 2026 02:46
Mutex lock;
ConditionVariable cv;
bool done = false;
Thread timer([&]{

@Mysticial Mysticial Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You cannot do this because it will hang on Windows due to Qt 6.9 bug.

We need to discuss what you're trying to do here, because this looks very ugly and I don't think this actually does what you want.

// Queued after the cancel, so the device reports this no-op finished only
// after it has dropped everything before it and held neutral for 10 ms.
issue_nop(&scope, Milliseconds(10));
wait_for_all(&scope);

@Mysticial Mysticial Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issuing a nop after the cancel and waiting for is probably the quick-and-easy way to do it. Not sure why you need a thread here.

Looking a bit more into this since I was a bit confused. PABotBase1 would directly ack the cancels so you would wait for that ack. But the cancel functions never waited for the ack because they were often called on destruction paths which would hang if the controller died.

PABotBase2 changes the architecture in a way that it would be "successful" as soon as it was committed into the reliable send queue where it "effectively guaranteed" to be received by the controller. There's no separate ack from the controller.

The way you would go about waiting for the controller to truly ack it without sending a dummy command is by calling ReliableStreamConnection::wait_for_pending(). But I haven't looked to see how much drilling we'd need through how many APIs to properly implement a blocking cancel via this path.

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