From 113b98e021fe50e910308695a57702cb8b25dfa3 Mon Sep 17 00:00:00 2001 From: Robert Zieba Date: Thu, 26 Feb 2026 10:08:13 -0800 Subject: [PATCH 1/2] type-c-service: Don't restart sink ready timeout if contract is same In certain cases, we can receive repeated source capabilities messages. This will reset the sink ready timeout and lead to a situation where we are never able to sink from the source. Only restart the timer if we get a different contract. --- type-c-service/src/wrapper/mod.rs | 5 ++- type-c-service/src/wrapper/pd.rs | 51 ++++++++++++++++++++----------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/type-c-service/src/wrapper/mod.rs b/type-c-service/src/wrapper/mod.rs index 38dcb170..87940ac2 100644 --- a/type-c-service/src/wrapper/mod.rs +++ b/type-c-service/src/wrapper/mod.rs @@ -262,7 +262,10 @@ where state, &status, local_port_id, - status_event.new_power_contract_as_consumer(), + status_event + .new_power_contract_as_consumer() + .then_some(status.available_sink_contract) + .flatten(), status_event.sink_ready(), )?; diff --git a/type-c-service/src/wrapper/pd.rs b/type-c-service/src/wrapper/pd.rs index f55bd8bb..086fed03 100644 --- a/type-c-service/src/wrapper/pd.rs +++ b/type-c-service/src/wrapper/pd.rs @@ -2,6 +2,7 @@ use embassy_futures::yield_now; use embassy_sync::pubsub::WaitResult; use embassy_time::{Duration, Timer}; use embedded_services::debug; +use embedded_services::power::policy::PowerCapability; use embedded_services::type_c::Cached; use embedded_services::type_c::controller::{InternalResponseData, Response}; use embedded_usb_pd::constants::{T_PS_TRANSITION_EPR_MS, T_PS_TRANSITION_SPR_MS}; @@ -48,36 +49,50 @@ where state: &mut dyn DynPortState<'_>, status: &PortStatus, port: LocalPortId, - new_contract: bool, + new_contract: Option, sink_ready: bool, ) -> Result<(), PdError> { if port.0 as usize >= state.num_ports() { return Err(PdError::InvalidPort); } - let deadline = &mut state + let port_state = state .port_states_mut() .get_mut(port.0 as usize) - .ok_or(PdError::InvalidPort)? - .sink_ready_deadline; + .ok_or(PdError::InvalidPort)?; - if new_contract && !sink_ready { - // Start the timeout - // Double the spec maximum transition time to provide a safety margin for hardware/controller delays our out-of-spec controllers. - let timeout_ms = if status.epr { - T_PS_TRANSITION_EPR_MS + let available_sink_contract = port_state.status.available_sink_contract; + let deadline = &mut port_state.sink_ready_deadline; + + // Check if we need to start the timeout + if let Some(new_contract) = new_contract { + let contract_changed = Some(new_contract) != available_sink_contract; + + // Don't start the timeout if the sink is already ready or if the contract didn't change. + // The latter ensures that soft resets won't continually reset the ready timeout + if !sink_ready && contract_changed { + // Start the timeout + // Double the spec maximum transition time to provide a safety margin for hardware/controller delays our out-of-spec controllers. + let timeout_ms = if status.epr { + T_PS_TRANSITION_EPR_MS + } else { + T_PS_TRANSITION_SPR_MS + } + .maximum + .0 * 2; + + debug!("Port{}: Sink ready timeout started for {}ms", port.0, timeout_ms); + *deadline = Some(Instant::now() + Duration::from_millis(timeout_ms as u64)); } else { - T_PS_TRANSITION_SPR_MS + debug!( + "Port{}: Not starting sink ready timeout, sink_ready: {}, contract_changed: {}", + port.0, sink_ready, contract_changed + ); } - .maximum - .0 * 2; + } - debug!("Port{}: Sink ready timeout started for {}ms", port.0, timeout_ms); - *deadline = Some(Instant::now() + Duration::from_millis(timeout_ms as u64)); - } else if deadline.is_some() - && (!status.is_connected() || status.available_sink_contract.is_none() || sink_ready) - { - // Clear the timeout + // Check if we need to clear the timeout + if deadline.is_some() && (!status.is_connected() || status.available_sink_contract.is_none() || sink_ready) { debug!("Port{}: Sink ready timeout cleared", port.0); *deadline = None; } From b0be4e739686aed7d2924b5b545acda5a4ceda81 Mon Sep 17 00:00:00 2001 From: Robert Zieba Date: Fri, 27 Feb 2026 11:09:57 -0800 Subject: [PATCH 2/2] Clean-up conditionals --- type-c-service/src/wrapper/mod.rs | 5 +--- type-c-service/src/wrapper/pd.rs | 50 +++++++++++++------------------ 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/type-c-service/src/wrapper/mod.rs b/type-c-service/src/wrapper/mod.rs index 87940ac2..38dcb170 100644 --- a/type-c-service/src/wrapper/mod.rs +++ b/type-c-service/src/wrapper/mod.rs @@ -262,10 +262,7 @@ where state, &status, local_port_id, - status_event - .new_power_contract_as_consumer() - .then_some(status.available_sink_contract) - .flatten(), + status_event.new_power_contract_as_consumer(), status_event.sink_ready(), )?; diff --git a/type-c-service/src/wrapper/pd.rs b/type-c-service/src/wrapper/pd.rs index 086fed03..5564c3b2 100644 --- a/type-c-service/src/wrapper/pd.rs +++ b/type-c-service/src/wrapper/pd.rs @@ -2,7 +2,6 @@ use embassy_futures::yield_now; use embassy_sync::pubsub::WaitResult; use embassy_time::{Duration, Timer}; use embedded_services::debug; -use embedded_services::power::policy::PowerCapability; use embedded_services::type_c::Cached; use embedded_services::type_c::controller::{InternalResponseData, Response}; use embedded_usb_pd::constants::{T_PS_TRANSITION_EPR_MS, T_PS_TRANSITION_SPR_MS}; @@ -49,7 +48,7 @@ where state: &mut dyn DynPortState<'_>, status: &PortStatus, port: LocalPortId, - new_contract: Option, + new_contract: bool, sink_ready: bool, ) -> Result<(), PdError> { if port.0 as usize >= state.num_ports() { @@ -61,38 +60,31 @@ where .get_mut(port.0 as usize) .ok_or(PdError::InvalidPort)?; - let available_sink_contract = port_state.status.available_sink_contract; + let contract_changed = port_state.status.available_sink_contract != status.available_sink_contract; let deadline = &mut port_state.sink_ready_deadline; - // Check if we need to start the timeout - if let Some(new_contract) = new_contract { - let contract_changed = Some(new_contract) != available_sink_contract; - - // Don't start the timeout if the sink is already ready or if the contract didn't change. - // The latter ensures that soft resets won't continually reset the ready timeout - if !sink_ready && contract_changed { - // Start the timeout - // Double the spec maximum transition time to provide a safety margin for hardware/controller delays our out-of-spec controllers. - let timeout_ms = if status.epr { - T_PS_TRANSITION_EPR_MS - } else { - T_PS_TRANSITION_SPR_MS - } - .maximum - .0 * 2; - - debug!("Port{}: Sink ready timeout started for {}ms", port.0, timeout_ms); - *deadline = Some(Instant::now() + Duration::from_millis(timeout_ms as u64)); + // Don't start the timeout if the sink has signaled it's ready or if the contract didn't change. + // The latter ensures that soft resets won't continually reset the ready timeout + debug!( + "Port{}: Check sink ready: new_contract={:?}, sink_ready={:?}, contract_changed={:?}, deadline={:?}", + port.0, new_contract, sink_ready, contract_changed, deadline, + ); + if new_contract && !sink_ready && contract_changed { + // Start the timeout + // Double the spec maximum transition time to provide a safety margin for hardware/controller delays or out-of-spec controllers. + let timeout_ms = if status.epr { + T_PS_TRANSITION_EPR_MS } else { - debug!( - "Port{}: Not starting sink ready timeout, sink_ready: {}, contract_changed: {}", - port.0, sink_ready, contract_changed - ); + T_PS_TRANSITION_SPR_MS } - } + .maximum + .0 * 2; - // Check if we need to clear the timeout - if deadline.is_some() && (!status.is_connected() || status.available_sink_contract.is_none() || sink_ready) { + debug!("Port{}: Sink ready timeout started for {}ms", port.0, timeout_ms); + *deadline = Some(Instant::now() + Duration::from_millis(timeout_ms as u64)); + } else if deadline.is_some() + && (!status.is_connected() || status.available_sink_contract.is_none() || sink_ready) + { debug!("Port{}: Sink ready timeout cleared", port.0); *deadline = None; }