type-c-service/tps6699x: Fix sink PDO ordering bug

embedded-service/src/type_c/controller.rs

@@ -35,7 +35,7 @@ pub struct PdSourceInfo {
     /// Received 5V sink PDO data from port partner
     ///
     /// Contains various flags that aren't present in other PDOs
-    pub rx_fixed_5v_data: pdo::sink::FixedData,
+    pub rx_fixed_5v_data: Option<pdo::sink::FixedData>,
     /// PDO associated with this contract
     pub pdo: pdo::source::Pdo,
     /// RDO associated with this contract
@@ -55,34 +55,43 @@ pub struct SourceContract {
 impl SourceContract {
     /// Returns true if the port partner has dual role power capability
     pub fn dual_role_power(&self) -> bool {
-        self.pd.map(|pd| pd.rx_fixed_5v_data.dual_role_power).unwrap_or(false)
+        self.pd
+            .and_then(|pd| pd.rx_fixed_5v_data.map(|data| data.dual_role_power))
+            .unwrap_or(false)
     }
 
     /// Returns true if the port partner has higher capability
     pub fn higher_capability(&self) -> bool {
-        self.pd.map(|pd| pd.rx_fixed_5v_data.higher_capability).unwrap_or(false)
+        self.pd
+            .and_then(|pd| pd.rx_fixed_5v_data.map(|data| data.higher_capability))
+            .unwrap_or(false)
     }
 
     /// Returns true if the port partner has unconstrained power
     pub fn unconstrained_power(&self) -> bool {
         self.pd
-            .map(|pd| pd.rx_fixed_5v_data.unconstrained_power)
+            .and_then(|pd| pd.rx_fixed_5v_data.map(|data| data.unconstrained_power))
             .unwrap_or(false)
     }
 
     /// Returns true if the port partner is USB comms capable
     pub fn usb_comms_capable(&self) -> bool {
-        self.pd.map(|pd| pd.rx_fixed_5v_data.usb_comms_capable).unwrap_or(false)
+        self.pd
+            .and_then(|pd| pd.rx_fixed_5v_data.map(|data| data.usb_comms_capable))
+            .unwrap_or(false)
     }
 
     /// Returns true if the port partner has dual role data capability
     pub fn dual_role_data(&self) -> bool {
-        self.pd.map(|pd| pd.rx_fixed_5v_data.dual_role_data).unwrap_or(false)
+        self.pd
+            .and_then(|pd| pd.rx_fixed_5v_data.map(|data| data.dual_role_data))
+            .unwrap_or(false)
     }
 
     /// Returns required FRS current for the port partner, if supported
     pub fn frs_required_current(&self) -> Option<FrsRequiredCurrent> {
-        self.pd.map(|pd| pd.rx_fixed_5v_data.frs_required_current)
+        self.pd
+            .and_then(|pd| pd.rx_fixed_5v_data.map(|data| data.frs_required_current))
     }
 }
 

examples/std/src/lib/type_c/mock_controller.rs

@@ -59,11 +59,11 @@ impl ControllerState {
                             max_operating_current_ma: capability.current_ma,
                             ..Default::default()
                         }),
-                        rx_fixed_5v_data: pdo::sink::FixedData {
+                        rx_fixed_5v_data: Some(pdo::sink::FixedData {
                             voltage_mv: capability.voltage_mv,
                             operational_current_ma: capability.current_ma,
                             ..Default::default()
-                        },
+                        }),
                     }),
                 });
             }

type-c-service/src/driver/tps6699x.rs

@@ -353,15 +353,18 @@ impl<M: RawMutex, B: I2c> Controller for Tps6699x<'_, M, B> {
                         .get_rx_snk_caps(port, &mut sink_pdos[..], &mut [])
                         .await?;
 
-                    if num_sprs == 0 {
-                        // USB PD spec requires at least one sink PDO be present, something is really wrong
-                        error!("Port{} no source PDOs found", port.0);
-                        return Err(PdError::InvalidParams.into());
-                    }
-
-                    let sink::Pdo::Fixed(rx_fixed_5v_data) = sink_pdos[0] else {
-                        error!("Port{}: First rx sink PDO is not fixed", port.0);
-                        return Err(PdError::InvalidParams.into());
+                    // Might not have received sink PDOs yet, but we can still report the contract.
+                    // Our fixed 5V flags will just be set to none until we receive the sink PDOs.
+                    let rx_fixed_5v_data = if num_sprs > 0 {
+                        if let sink::Pdo::Fixed(data) = sink_pdos[0] {
+                            Some(data)
+                        } else {
+                            error!("Port{}: First rx sink PDO is not fixed", port.0);
+                            return Err(PdError::InvalidParams.into());
+                        }
+                    } else {
+                        debug!("Port{}: No rx sink PDOs received yet", port.0);
+                        None
                     };
 
                     let pdo = source::Pdo::try_from(pdo_raw).map_err(|_| Error::from(PdError::InvalidParams))?;
@@ -391,9 +394,10 @@ impl<M: RawMutex, B: I2c> Controller for Tps6699x<'_, M, B> {
                         .get_rx_src_caps(port, &mut source_pdos[..], &mut [])
                         .await?;
 
+                    // We have received source PDOs at this point because otherwise we wouldn't have been able to negotiate a contract.
                     if num_sprs == 0 {
                         // USB PD spec requires at least one source PDO be present, something is really wrong
-                        error!("Port{} no source PDOs found", port.0);
+                        error!("Port{} no source PDOs received", port.0);
                         return Err(PdError::InvalidParams.into());
                     }
 

type-c-service/tests/common/mock.rs

@@ -218,11 +218,11 @@ impl<'a> ControllerState<'a> {
                             max_operating_current_ma: capability.current_ma,
                             ..Default::default()
                         }),
-                        rx_fixed_5v_data: pdo::sink::FixedData {
+                        rx_fixed_5v_data: Some(pdo::sink::FixedData {
                             voltage_mv: capability.voltage_mv,
                             operational_current_ma: capability.current_ma,
                             ..Default::default()
-                        },
+                        }),
                     }),
                 });
             }