feat(rdpeusb): implement urbdrc client#1365
Open
uchouT (uchouT) wants to merge 3 commits into
Open
Conversation
uchouT (uchouT)
force-pushed
the
urbdrc-client
branch
from
6bfdada to
9b3f945
Compare
uchouT (uchouT)
force-pushed
the
urbdrc-client
branch
from
97479e8 to
acacd98
Compare
Contributor
Author
|
Marc-Andre Lureau (@elmarco) Benoît Cortier (@CBenoit) [EDIT]: |
uchouT (uchouT)
changed the title
uchouT (uchouT)
changed the title
uchouT (uchouT)
force-pushed
the
urbdrc-client
branch
10 times, most recently
from
c25c053 to
b478576
Compare
There was a problem hiding this comment.
⚠️ Not ready to approve
The client state machine currently diverges from the issue’s required startup sequencing and adds substantial new protocol logic without the acceptance-criteria unit tests.
Pull request overview
Adds an ironrdp-rdpeusb client-side implementation of the URBDRC dynamic virtual channel, including a control-channel processor, per-device channel processor, and backend-neutral device metadata conversion used to construct RDPEUSB PDUs.
Changes:
- Introduces
UrbdrcListener,UrbdrcControlClient, andUrbdrcDeviceClientwith a backend trait surface and pending-IO completion helpers. - Adds backend-neutral
DeviceInfo→ADD_DEVICEconversion (PnP-style IDs, container IDs, capabilities) following observed FreeRDP behavior. - Integrates RDPEUSB PDUs with the DVC layer by implementing
ironrdp_dvc::DvcEncodeand adding anironrdp-dvcdependency.
File summaries
| File | Description |
|---|---|
| crates/ironrdp-rdpeusb/src/pdu/usb_dev/mod.rs | Adds DvcEncode impls and a helper to extract transfer request IDs. |
| crates/ironrdp-rdpeusb/src/pdu/sink.rs | Marks sink PDUs as DVC-encodable (DvcEncode). |
| crates/ironrdp-rdpeusb/src/pdu/notify.rs | Marks CHANNEL_CREATED as DVC-encodable (DvcEncode). |
| crates/ironrdp-rdpeusb/src/pdu/iface_manipulation.rs | Marks interface-manipulation PDUs as DVC-encodable (DvcEncode). |
| crates/ironrdp-rdpeusb/src/pdu/header.rs | Exposes InterfaceId::from_raw within the crate for client allocation. |
| crates/ironrdp-rdpeusb/src/pdu/completion/mod.rs | Marks completion PDUs as DVC-encodable (DvcEncode). |
| crates/ironrdp-rdpeusb/src/pdu/caps.rs | Marks capability PDUs as DVC-encodable (DvcEncode). |
| crates/ironrdp-rdpeusb/src/lib.rs | Adds channel name, exports client module, and introduces InvalidDeviceInterfaceId error type. |
| crates/ironrdp-rdpeusb/src/client/mod.rs | Implements the URBDRC listener + control/device client processors and backend traits. |
| crates/ironrdp-rdpeusb/src/client/device.rs | Implements backend-neutral USB device facts and ADD_DEVICE PDU construction. |
| crates/ironrdp-rdpeusb/Cargo.toml | Adds public dependency on ironrdp-dvc. |
| Cargo.lock | Records the new ironrdp-dvc dependency edge. |
Copilot's findings
- Files reviewed: 11/12 changed files
- Comments generated: 8
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+67
to
+90
| const fn alloc(&mut self) -> InterfaceId { | ||
| self.id += 1; | ||
| if self.id > 0x3F_FF_FF_FF { | ||
| panic!("USB device amount overflow") | ||
| } | ||
| InterfaceId::from_raw(self.id) | ||
| } | ||
| } | ||
|
|
||
| impl DvcChannelListener for UrbdrcListener { | ||
| fn channel_name(&self) -> &str { | ||
| CHANNEL_NAME | ||
| } | ||
|
|
||
| fn create(&mut self, channel_id: u32) -> Option<Box<dyn DvcProcessor>> { | ||
| if let Some(callback) = self.on_capability_exchanged.take() { | ||
| self.device_man.control_channel_assigned(channel_id); | ||
| Some(Box::new(UrbdrcControlClient::new(callback))) | ||
| } else { | ||
| #[expect(clippy::as_conversions)] | ||
| self.device_man.take_device_for_channel(channel_id).map(|backend| { | ||
| Box::new(UrbdrcDeviceClient::new(self.iface_man.alloc(), backend).expect("invalid interface id")) | ||
| as Box<dyn DvcProcessor> | ||
| }) |
uchouT (uchouT)
force-pushed
the
urbdrc-client
branch
from
8303938 to
06f93df
Compare
Signed-off-by: uchouT <i@uchout.moe>
`ADD_DEVICE` message is constructed following FreeRDP's pattern. `UrbdrcDeviceClient` maintains a pending IO request map. When the packet is handled asynchronously, the information for the completion message construction (such as output buffer size) will be stored in this map, and `UrbdrcDeviceClient` provides public APIs for completion message construction, which will consume the map items internally. Signed-off-by: uchouT <i@uchout.moe>
Signed-off-by: uchouT <i@uchout.moe> Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
uchouT (uchouT)
force-pushed
the
urbdrc-client
branch
from
06f93df to
6c2eaaf
Compare
Contributor
Author
|
Updated |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1137
The device dvc processor manages a
pending_iomap internally and offers API for upper layer to construct completionDvcMessage.