From 8bf93d647bc0762e9236d3a89b55b94e1ed3d952 Mon Sep 17 00:00:00 2001 From: Robert Zieba Date: Mon, 19 Oct 2026 10:29:18 -0700 Subject: [PATCH 1/2] Adopt public fields for UCSI types embedded-usb-pd now exposes UCSI struct fields directly instead of through getters and setters, and LPM commands carry the connector number in their arguments. Update tcpm-service, its tests and the std example to use the fields, and retarget LPM commands to the local port with MapPort. Assisted-by: GitHub Copilot:claude-sonnet-5.5 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c9a94a93-729f-451c-b08e-78887f5542e8 --- Cargo.lock | 43 +++-- examples/std/Cargo.lock | 90 ++++++---- .../std/src/lib/type_c/mock_controller.rs | 7 +- supply-chain/imports.lock | 38 +++-- tcpm-service/src/service/power.rs | 4 +- tcpm-service/src/service/ucsi.rs | 59 ++++--- tcpm-service/tests/ucsi.rs | 160 +++++++++++------- 7 files changed, 237 insertions(+), 164 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 043539aa..7b66d166 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -160,15 +160,6 @@ dependencies = [ "maitake-sync", ] -[[package]] -name = "bincode" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "36eaf5d7b090263e8150820482d5d93cd964a81e4019913c972f4edcc6edb740" -dependencies = [ - "unty", -] - [[package]] name = "bit-register" version = "0.1.0" @@ -260,6 +251,20 @@ name = "bytemuck" version = "1.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" +dependencies = [ + "bytemuck_derive", +] + +[[package]] +name = "bytemuck_derive" +version = "1.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f9abbd1bc6865053c427f7198e6af43bfdedc55ab791faed4fbd361d789575ff" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] [[package]] name = "byteorder" @@ -818,12 +823,13 @@ dependencies = [ [[package]] name = "embedded-usb-pd" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#071f163395e3eaea3094b817e3d943ab7265be3d" +source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#87a012b36fec7257d915a9896b5633f3aa070d79" dependencies = [ - "bincode", "bitfield 0.19.4", + "bytemuck", "defmt 0.3.100", "embedded-hal-async", + "pack1", ] [[package]] @@ -1382,6 +1388,15 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "pack1" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3b7bb0ecf2e447b1f20ee94ee79ef6eed1e9d4b3c36ce1903b9dea3bf205523" +dependencies = [ + "bytemuck", +] + [[package]] name = "partition-manager" version = "0.1.0" @@ -2230,12 +2245,6 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" -[[package]] -name = "unty" -version = "0.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d49784317cd0d1ee7ec5c716dd598ec5b4483ea832a2dced265471cc0f690ae" - [[package]] name = "usb-device" version = "0.3.2" diff --git a/examples/std/Cargo.lock b/examples/std/Cargo.lock index e243b3b2..6c2af0b8 100644 --- a/examples/std/Cargo.lock +++ b/examples/std/Cargo.lock @@ -109,15 +109,6 @@ dependencies = [ "maitake-sync", ] -[[package]] -name = "bincode" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "36eaf5d7b090263e8150820482d5d93cd964a81e4019913c972f4edcc6edb740" -dependencies = [ - "unty", -] - [[package]] name = "bit-register" version = "0.1.0" @@ -163,7 +154,7 @@ checksum = "f48d6ace212fdf1b45fd6b566bb40808415344642b76c3224c07c8df9da81e97" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -174,7 +165,7 @@ checksum = "8769c4854c5ada2852ddf6fd09d15cf43d4c2aaeccb4de6432f5402f08a6003b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -189,6 +180,26 @@ version = "2.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +[[package]] +name = "bytemuck" +version = "1.25.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95832e849adfb21180ccb6826a99da14e5d266ae5c2e668e1602cf234f153797" +dependencies = [ + "bytemuck_derive", +] + +[[package]] +name = "bytemuck_derive" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a1f896587b6f2c069c73d2f0913e2d590c3990285cd2f0b6aa02b786b4c679c" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.9", +] + [[package]] name = "byteorder" version = "1.5.0" @@ -286,7 +297,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn", + "syn 2.0.117", ] [[package]] @@ -297,7 +308,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -351,7 +362,7 @@ dependencies = [ "proc-macro-error2", "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -395,7 +406,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -612,11 +623,12 @@ dependencies = [ [[package]] name = "embedded-usb-pd" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#071f163395e3eaea3094b817e3d943ab7265be3d" +source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#87a012b36fec7257d915a9896b5633f3aa070d79" dependencies = [ - "bincode", "bitfield 0.19.4", + "bytemuck", "embedded-hal-async", + "pack1", ] [[package]] @@ -770,7 +782,7 @@ checksum = "e000de030ff8022ea1da3f466fbb0f3a809f5e51ed31f6dd931c35181ad8e6d7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -914,7 +926,7 @@ checksum = "680998035259dcfcafe653688bf2aa6d3e2dc05e98be6ab46afb089dc84f1df8" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -937,6 +949,15 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "pack1" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3b7bb0ecf2e447b1f20ee94ee79ef6eed1e9d4b3c36ce1903b9dea3bf205523" +dependencies = [ + "bytemuck", +] + [[package]] name = "paste" version = "1.0.15" @@ -960,7 +981,7 @@ checksum = "c96395f0a926bc13b1c17622aaddda1ecb55d49c8f1bf9777e4d877800a43f8b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1026,7 +1047,7 @@ dependencies = [ "proc-macro-error-attr2", "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1150,7 +1171,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1248,7 +1269,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1262,6 +1283,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d78c8dee4c7bf0e14673097256fed6142ce9d3b85a408189d07482442145823b" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "tcpm-interface" version = "0.1.0" @@ -1332,7 +1364,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1363,7 +1395,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] [[package]] @@ -1423,12 +1455,6 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" -[[package]] -name = "unty" -version = "0.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d49784317cd0d1ee7ec5c716dd598ec5b4483ea832a2dced265471cc0f690ae" - [[package]] name = "utf8parse" version = "0.2.2" @@ -1503,5 +1529,5 @@ checksum = "70e3cd084b1788766f53af483dd21f93881ff30d7320490ec3ef7526d203bad4" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.117", ] diff --git a/examples/std/src/lib/type_c/mock_controller.rs b/examples/std/src/lib/type_c/mock_controller.rs index 5eef8b10..f285aadb 100644 --- a/examples/std/src/lib/type_c/mock_controller.rs +++ b/examples/std/src/lib/type_c/mock_controller.rs @@ -320,9 +320,12 @@ impl tcpm_interface::controller::type_c::StateMachine for Controller<'_> { impl tcpm_interface::ucsi::Lpm for Controller<'_> { async fn execute_lpm_command(&mut self, command: lpm::LocalCommand) -> Result, PdError> { - debug!("Execute UCSI command for port {:?}: {command:?}", command.port()); + debug!( + "Execute UCSI command for port {:?}: {command:?}", + command.connector_number() + ); match command.operation() { - lpm::CommandData::GetConnectorStatus => Ok(Some(lpm::ResponseData::GetConnectorStatus( + lpm::CommandData::GetConnectorStatus(_) => Ok(Some(lpm::ResponseData::GetConnectorStatus( lpm::get_connector_status::ResponseData::default(), ))), _ => Err(PdError::UnrecognizedCommand), diff --git a/supply-chain/imports.lock b/supply-chain/imports.lock index 93d99aa4..93adccea 100644 --- a/supply-chain/imports.lock +++ b/supply-chain/imports.lock @@ -151,6 +151,16 @@ criteria = "safe-to-deploy" version = "0.13.2" aggregated-from = "https://raw.githubusercontent.com/OpenDevicePartnership/mcxa-pac/refs/heads/main/supply-chain/audits.toml" +[[audits.OpenDevicePartnership.audits.bytemuck_derive]] +who = "Robert Zieba " +criteria = "safe-to-deploy" +version = "1.10.2" +notes = "Changes support more types of structs and enums, no unsafe changes to the generated code." +aggregated-from = [ + "https://raw.githubusercontent.com/OpenDevicePartnership/tps6699x/refs/heads/main/supply-chain/audits.toml", + "https://raw.githubusercontent.com/OpenDevicePartnership/embedded-usb-pd/refs/heads/main/supply-chain/audits.toml", +] + [[audits.OpenDevicePartnership.audits.cortex-m]] who = "Felipe Balbi " criteria = "safe-to-deploy" @@ -232,6 +242,16 @@ criteria = "safe-to-deploy" delta = "0.2.1 -> 0.2.2" aggregated-from = "https://raw.githubusercontent.com/OpenDevicePartnership/embassy-imxrt/refs/heads/main/supply-chain/audits.toml" +[[audits.OpenDevicePartnership.audits.pack1]] +who = "Robert Zieba " +criteria = "safe-to-deploy" +version = "1.1.0" +notes = "No unsafe code, only wrapper types around byte arrays, endianness conversion are handled by the built-in `from_*_bytes` functions." +aggregated-from = [ + "https://raw.githubusercontent.com/OpenDevicePartnership/tps6699x/refs/heads/main/supply-chain/audits.toml", + "https://raw.githubusercontent.com/OpenDevicePartnership/embedded-usb-pd/refs/heads/main/supply-chain/audits.toml", +] + [[audits.OpenDevicePartnership.audits.proc-macro-error-attr2]] who = "Felipe Balbi " criteria = "safe-to-deploy" @@ -393,24 +413,6 @@ criteria = "safe-to-deploy" delta = "0.4.1 -> 0.5.0" notes = "Minor changes for a `no_std` upgrade but otherwise everything looks as expected." -[[audits.bytecode-alliance.audits.itertools]] -who = "Nick Fitzgerald " -criteria = "safe-to-deploy" -delta = "0.10.5 -> 0.12.1" -notes = """ -Minimal `unsafe` usage. Few blocks that existed looked reasonable. Does what it -says on the tin: lots of iterators. -""" - -[[audits.bytecode-alliance.audits.itertools]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "0.12.1 -> 0.14.0" -notes = """ -Lots of new iterators and shuffling some things around. Some new unsafe code but -it's well-documented and well-tested. Nothing suspicious. -""" - [[audits.bytecode-alliance.audits.matchers]] who = "Pat Hickey " criteria = "safe-to-deploy" diff --git a/tcpm-service/src/service/power.rs b/tcpm-service/src/service/power.rs index 06d879e4..b412e700 100644 --- a/tcpm-service/src/service/power.rs +++ b/tcpm-service/src/service/power.rs @@ -76,7 +76,7 @@ impl<'a, Reg: Registration<'a>> Service<'a, Reg> { PowerPolicyEventData::ConsumerDisconnected(_) => { self.ucsi.psu_connected = false; // Notify OPM because this can affect battery charging capability status - if self.ucsi.notifications_enabled.battery_charge_change() { + if self.ucsi.notifications_enabled.battery_charge_change { self.pend_ucsi_connected_ports().await; } Ok(()) @@ -84,7 +84,7 @@ impl<'a, Reg: Registration<'a>> Service<'a, Reg> { PowerPolicyEventData::ConsumerConnected(_) => { self.ucsi.psu_connected = true; // Notify OPM because this can affect battery charging capability status - if self.ucsi.notifications_enabled.battery_charge_change() { + if self.ucsi.notifications_enabled.battery_charge_change { self.pend_ucsi_connected_ports().await; } Ok(()) diff --git a/tcpm-service/src/service/ucsi.rs b/tcpm-service/src/service/ucsi.rs index f634e2e1..58c9f3e2 100644 --- a/tcpm-service/src/service/ucsi.rs +++ b/tcpm-service/src/service/ucsi.rs @@ -1,5 +1,6 @@ use embedded_services::sync::Lockable; use embedded_services::warn; +use embedded_usb_pd::ucsi::MapPort as _; use embedded_usb_pd::ucsi::v1_2::cci::{Cci, GlobalCci}; use embedded_usb_pd::ucsi::v1_2::lpm::get_connector_status::{BatteryChargingCapabilityStatus, ConnectorStatusChange}; use embedded_usb_pd::ucsi::v1_2::ppm::set_notification_enable::NotificationEnable; @@ -109,15 +110,16 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { command: &ucsi::lpm::GlobalCommand, ) -> Result, PdError> { debug!("Processing LPM command: {:?}", command); - let mut port = self.lookup_port(command.port())?.lock().await; + let global_port_id = command.connector_number(); + let mut port = self.lookup_port(global_port_id)?.lock().await; let local_port_id = self .registration - .ucsi_local_port_id(command.port()) + .ucsi_local_port_id(global_port_id) .ok_or(PdError::InvalidPort)?; - let local_command = ucsi::lpm::LocalCommand::new(local_port_id, command.operation()); + let local_command = command.map_port(|_| local_port_id); match command.operation() { - lpm::CommandData::GetConnectorCapability => { + lpm::CommandData::GetConnectorCapability(_) => { // Override the capabilities if present in the config if let Some(capabilities) = &self.config.ucsi_port_capabilities { Ok(Some(lpm::ResponseData::GetConnectorCapability(*capabilities))) @@ -125,7 +127,7 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { port.execute_lpm_command(local_command).await } } - lpm::CommandData::GetConnectorStatus => { + lpm::CommandData::GetConnectorStatus(_) => { let mut response = port.execute_lpm_command(local_command).await; if let Ok(Some(lpm::ResponseData::GetConnectorStatus(lpm::get_connector_status::ResponseData { status_change: ref mut states_change, @@ -139,8 +141,8 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { { let port_status = port.get_port_status().await?; *battery_charging_status = - self.determine_battery_charging_capability_status(command.port(), &port_status); - states_change.set_battery_charging_status_change(battery_charging_status.is_some()); + self.determine_battery_charging_capability_status(global_port_id, &port_status); + states_change.battery_charging_status_change = battery_charging_status.is_some(); } response @@ -153,10 +155,10 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { fn set_cci_connector_change(&self, cci: &mut GlobalCci) { if let Some(current_port) = self.ucsi.pending_ports.front() { // UCSI connector numbers are 1-based - cci.set_connector_change(GlobalPortId(current_port.0 + 1)); + cci.connector_change = GlobalPortId(current_port.0 + 1); } else { // 0 is used to indicate no pending connector changes - cci.set_connector_change(GlobalPortId(0)); + cci.connector_change = GlobalPortId(0); } } @@ -243,19 +245,19 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { // Don't return yet, need to inform state machine that command is complete } PpmOutput::OpmNotifyCommandComplete => { - response.notify_opm = self.ucsi.notifications_enabled.cmd_complete(); - response.cci.set_cmd_complete(true); - response.cci.set_error(response.data.is_err()); + response.notify_opm = self.ucsi.notifications_enabled.cmd_complete; + response.cci.cmd_complete = true; + response.cci.error = response.data.is_err(); self.set_cci_connector_change(&mut response.cci); return response; } PpmOutput::AckComplete(ack) => { - response.notify_opm = self.ucsi.notifications_enabled.cmd_complete(); - if ack.command_complete() { - response.cci.set_ack_command(true); + response.notify_opm = self.ucsi.notifications_enabled.cmd_complete; + if ack.command_complete { + response.cci.ack_command = true; } - if ack.connector_change() { + if ack.connector_change { self.ack_connector_change(&mut response.cci).await; } @@ -274,7 +276,7 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { PpmOutput::OpmNotifyBusy => { // Notify if notifications are enabled in general response.notify_opm = !self.ucsi.notifications_enabled.is_empty(); - response.cci.set_busy(true); + response.cci.busy = true; self.set_cci_connector_change(&mut response.cci); return response; } @@ -299,25 +301,26 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { port_event: PortStatusEventBitfield, port_status: &PortStatus, ) { - let mut ucsi_event = ConnectorStatusChange::default(); - - ucsi_event.set_connect_change(port_event.plug_inserted_or_removed()); - ucsi_event.set_power_direction_changed(port_event.power_swap_completed()); - ucsi_event.set_pd_reset_complete(port_event.pd_hard_reset()); + let mut ucsi_event = ConnectorStatusChange { + connect_change: port_event.plug_inserted_or_removed(), + power_direction_changed: port_event.power_swap_completed(), + pd_reset_complete: port_event.pd_hard_reset(), + ..Default::default() + }; if port_event.data_swap_completed() || port_event.alt_mode_entered() { - ucsi_event.set_connector_partner_changed(true); + ucsi_event.connector_partner_changed = true; } if port_event.new_power_contract_as_consumer() || port_event.new_power_contract_as_provider() || port_event.sink_ready() { - ucsi_event.set_negotiated_power_level_change(true); - ucsi_event.set_power_op_mode_change(true); - ucsi_event.set_external_supply_change(true); - ucsi_event.set_power_direction_changed(true); - ucsi_event.set_battery_charging_status_change(true); + ucsi_event.negotiated_power_level_change = true; + ucsi_event.power_op_mode_change = true; + ucsi_event.external_supply_change = true; + ucsi_event.power_direction_changed = true; + ucsi_event.battery_charging_status_change = true; // Power negotiation completed, battery charging capability status is now valid if self.ucsi.valid_battery_charging_capability.insert(port_id).is_err() { diff --git a/tcpm-service/tests/ucsi.rs b/tcpm-service/tests/ucsi.rs index 8836b7d3..7e4472b5 100644 --- a/tcpm-service/tests/ucsi.rs +++ b/tcpm-service/tests/ucsi.rs @@ -117,28 +117,19 @@ async fn test_lpm( info!("Testing LPM commands for port {port_id:?}"); // GetConnectorCapability is served from the config override, so no mock response is needed. - let expected_capability = LpmResponseData::GetConnectorCapability( - *UcsiConnectorCapability::default() - .set_operation_mode( - *OperationModeFlags::default() - .set_drp(true) - .set_usb2(true) - .set_usb3(true), - ) - .set_consumer(true) - .set_provider(true) - .set_swap_to_dfp(true) - .set_swap_to_snk(true) - .set_swap_to_src(true), - ); + let expected_capability = LpmResponseData::GetConnectorCapability(port_capability()); execute_and_assert( service, - GlobalCommand::LpmCommand(lpm::GlobalCommand::new( - port_id, - lpm::CommandData::GetConnectorCapability, - )), + GlobalCommand::LpmCommand(lpm::GlobalCommand::new(lpm::CommandData::GetConnectorCapability( + lpm::get_connector_capability::Args { + connector_number: port_id, + }, + ))), true, - *GlobalCci::default().set_cmd_complete(true), + GlobalCci { + cmd_complete: true, + ..Default::default() + }, Ok(Some(UcsiResponseData::Lpm(expected_capability))), ) .await; @@ -147,10 +138,16 @@ async fn test_lpm( execute_and_assert( service, GlobalCommand::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args { - ack: *Ack::default().set_command_complete(true), + ack: Ack { + command_complete: true, + ..Default::default() + }, })), true, - *GlobalCci::default().set_ack_command(true), + GlobalCci { + ack_command: true, + ..Default::default() + }, Ok(None), ) .await; @@ -162,9 +159,11 @@ async fn test_lpm( // GetConnectorStatus while connected. info!("Testing GetConnectorStatus for connected port {}", port_id.0); - let mut status_change = ConnectorStatusChange::default(); - status_change.set_connect_change(true); - status_change.set_battery_charging_status_change(true); + let status_change = ConnectorStatusChange { + connect_change: true, + battery_charging_status_change: true, + ..Default::default() + }; let connected_status = ConnectedStatus { battery_charging_status: Some(BatteryChargingCapabilityStatus::Nominal), ..Default::default() @@ -187,28 +186,21 @@ async fn test_lpm( } execute_and_assert( service, - GlobalCommand::LpmCommand(lpm::GlobalCommand::new(port_id, lpm::CommandData::GetConnectorStatus)), + get_connector_status_command(port_id), true, - *GlobalCci::default() - .set_cmd_complete(true) + GlobalCci { + cmd_complete: true, // UCSI connector numbers are 1-based. - .set_connector_change(GlobalPortId(port_id.0 + 1)), + connector_change: GlobalPortId(port_id.0 + 1), + ..Default::default() + }, Ok(Some(UcsiResponseData::Lpm(connected_response))), ) .await; // Acknowledge the connector change. info!("Acknowledging CCI for port {}", port_id.0); - execute_and_assert( - service, - GlobalCommand::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args { - ack: *Ack::default().set_command_complete(true).set_connector_change(true), - })), - true, - *GlobalCci::default().set_ack_command(true), - Ok(None), - ) - .await; + ack_command_and_connector_change(service).await; // Disconnect and verify the UCSI connector-change notification. info!("Disconnecting port {}", port_id.0); @@ -228,29 +220,66 @@ async fn test_lpm( .push_back(Ok(Some(disconnected_response))); execute_and_assert( service, - GlobalCommand::LpmCommand(lpm::GlobalCommand::new(port_id, lpm::CommandData::GetConnectorStatus)), + get_connector_status_command(port_id), true, - *GlobalCci::default() - .set_cmd_complete(true) - .set_connector_change(GlobalPortId(port_id.0 + 1)), + GlobalCci { + cmd_complete: true, + connector_change: GlobalPortId(port_id.0 + 1), + ..Default::default() + }, Ok(Some(UcsiResponseData::Lpm(disconnected_response))), ) .await; // Acknowledge the connector change. info!("Acknowledging CCI for port {}", port_id.0); + ack_command_and_connector_change(service).await; +} + +fn get_connector_status_command(port_id: GlobalPortId) -> GlobalCommand { + GlobalCommand::LpmCommand(lpm::GlobalCommand::new(lpm::CommandData::GetConnectorStatus( + lpm::get_connector_status::Args { + connector_number: port_id, + }, + ))) +} + +async fn ack_command_and_connector_change(service: &TypeCServiceMutexType<'_, '_>) { execute_and_assert( service, GlobalCommand::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args { - ack: *Ack::default().set_command_complete(true).set_connector_change(true), + ack: Ack { + command_complete: true, + connector_change: true, + }, })), true, - *GlobalCci::default().set_ack_command(true), + GlobalCci { + ack_command: true, + ..Default::default() + }, Ok(None), ) .await; } +fn port_capability() -> UcsiConnectorCapability { + UcsiConnectorCapability { + operation_mode: OperationModeFlags { + drp: true, + usb2: true, + usb3: true, + ..Default::default() + }, + consumer: true, + provider: true, + swap_to_dfp: true, + swap_to_snk: true, + swap_to_src: true, + ..Default::default() + } +} + struct TestUcsi; impl Test for TestUcsi { @@ -270,16 +299,21 @@ impl Test for TestUcsi { GlobalCommand::PpmCommand(ppm::Command::PpmReset), // OPM is expected to poll for the reset complete flag. false, - *GlobalCci::default().set_reset_complete(true), + GlobalCci { + reset_complete: true, + ..Default::default() + }, Ok(None), ) .await; // Enable notifications. info!("Enabling notifications"); - let mut notifications = NotificationEnable::default(); - notifications.set_cmd_complete(true); - notifications.set_connect_change(true); + let notifications = NotificationEnable { + cmd_complete: true, + connect_change: true, + ..Default::default() + }; execute_and_assert( service, GlobalCommand::PpmCommand(ppm::Command::SetNotificationEnable( @@ -288,7 +322,10 @@ impl Test for TestUcsi { }, )), true, - *GlobalCci::default().set_cmd_complete(true), + GlobalCci { + cmd_complete: true, + ..Default::default() + }, Ok(None), ) .await; @@ -297,10 +334,16 @@ impl Test for TestUcsi { execute_and_assert( service, GlobalCommand::PpmCommand(ppm::Command::AckCcCi(ppm::ack_cc_ci::Args { - ack: *Ack::default().set_command_complete(true), + ack: Ack { + command_complete: true, + ..Default::default() + }, })), true, - *GlobalCci::default().set_ack_command(true), + GlobalCci { + ack_command: true, + ..Default::default() + }, Ok(None), ) .await; @@ -323,20 +366,7 @@ async fn ucsi() { bcd_battery_charging_spec: 0x0120, ..Default::default() }, - ucsi_port_capabilities: Some( - *UcsiConnectorCapability::default() - .set_operation_mode( - *OperationModeFlags::default() - .set_drp(true) - .set_usb2(true) - .set_usb3(true), - ) - .set_consumer(true) - .set_provider(true) - .set_swap_to_dfp(true) - .set_swap_to_snk(true) - .set_swap_to_src(true), - ), + ucsi_port_capabilities: Some(port_capability()), ..Default::default() }, Default::default(), From 14efe9d63b71aa2521d6501443e79fa6ea4c96e5 Mon Sep 17 00:00:00 2001 From: Robert Zieba Date: Tue, 27 Oct 2026 08:25:24 -0700 Subject: [PATCH 2/2] Pull in more `embedded-usb-pd` changes, add TODO for `GET_ERROR_STATUS` --- Cargo.lock | 2 +- examples/std/Cargo.lock | 2 +- examples/std/src/lib/type_c/mock_controller.rs | 1 + tcpm-service/src/service/ucsi.rs | 6 ++++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7b66d166..e163f9b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -823,7 +823,7 @@ dependencies = [ [[package]] name = "embedded-usb-pd" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#87a012b36fec7257d915a9896b5633f3aa070d79" +source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#a9088fc8549068c033cb4587b604b2573682dbf1" dependencies = [ "bitfield 0.19.4", "bytemuck", diff --git a/examples/std/Cargo.lock b/examples/std/Cargo.lock index 6c2af0b8..2d6c4a2d 100644 --- a/examples/std/Cargo.lock +++ b/examples/std/Cargo.lock @@ -623,7 +623,7 @@ dependencies = [ [[package]] name = "embedded-usb-pd" version = "0.1.0" -source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#87a012b36fec7257d915a9896b5633f3aa070d79" +source = "git+https://github.com/OpenDevicePartnership/embedded-usb-pd#a9088fc8549068c033cb4587b604b2573682dbf1" dependencies = [ "bitfield 0.19.4", "bytemuck", diff --git a/examples/std/src/lib/type_c/mock_controller.rs b/examples/std/src/lib/type_c/mock_controller.rs index f285aadb..de2c663d 100644 --- a/examples/std/src/lib/type_c/mock_controller.rs +++ b/examples/std/src/lib/type_c/mock_controller.rs @@ -10,6 +10,7 @@ use embedded_usb_pd::{PowerRole, type_c::Current}; use embedded_usb_pd::{type_c::ConnectionState, ucsi::v1_2::lpm}; use log::{debug, info}; +use embedded_usb_pd::ucsi::ConnectorNumber; use power_policy_interface::capability::PowerCapability; use tcpm_interface::control::dp::{DpConfig, DpPinConfig, DpStatus}; use tcpm_interface::control::pd::{ diff --git a/tcpm-service/src/service/ucsi.rs b/tcpm-service/src/service/ucsi.rs index 58c9f3e2..feab7530 100644 --- a/tcpm-service/src/service/ucsi.rs +++ b/tcpm-service/src/service/ucsi.rs @@ -1,5 +1,6 @@ use embedded_services::sync::Lockable; use embedded_services::warn; +use embedded_usb_pd::ucsi::ConnectorNumber as _; use embedded_usb_pd::ucsi::MapPort as _; use embedded_usb_pd::ucsi::v1_2::cci::{Cci, GlobalCci}; use embedded_usb_pd::ucsi::v1_2::lpm::get_connector_status::{BatteryChargingCapabilityStatus, ConnectorStatusChange}; @@ -75,6 +76,7 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { Ok(None) } ppm::Command::GetCapability => Ok(Some(self.process_get_capabilities())), + // TODO(https://github.com/OpenDevicePartnership/embedded-services/issues/977): Implement `GET_ERROR_STATUS` PPM command _ => Ok(None), // Other commands are currently no-ops } } @@ -206,7 +208,7 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { error!("Unexpected end of state machine processing"); return UcsiResponse { notify_opm: true, - cci: Cci::new_error(), + cci: Cci::new_command_error(), data: Err(PdError::InvalidMode), }; }; @@ -217,7 +219,7 @@ impl<'port, Reg: Registration<'port>> Service<'port, Reg> { error!("PPM state machine transition failed: {:#?}", e); return UcsiResponse { notify_opm: true, - cci: Cci::new_error(), + cci: Cci::new_command_error(), data: Err(PdError::Failed), }; }