type-c-service: Don't restart sink ready timeout if contract is same#737
type-c-service: Don't restart sink ready timeout if contract is same#737RobertZ2011 wants to merge 1 commit intoOpenDevicePartnership:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the Type-C service sink-ready timeout logic to avoid perpetually delaying sink readiness when repeated capabilities/contract notifications arrive with no actual contract change.
Changes:
- Change
check_sink_ready_timeoutto accept the newly negotiated sink contract (Option<PowerCapability>) rather than a boolean “new contract” flag. - Only (re)start the sink-ready timeout when the contract value differs from the previously cached contract.
- Update the wrapper event path to pass through the optional contract when a new consumer contract event is observed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| type-c-service/src/wrapper/pd.rs | Compares the incoming contract to cached state to decide whether to restart the sink-ready deadline, and adds logging around the decision. |
| type-c-service/src/wrapper/mod.rs | Updates the call site to provide the optional PowerCapability only when a new consumer contract event occurs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
RobertZ2011
force-pushed
the
type-c-sink-ready-change
branch
from
4164f6d to
113b98e
Compare
RobertZ2011
requested review from
asasine,
Copilot,
felipebalbi,
gjpmsft,
jerrysxie,
kurtjd and
tullom
and removed request for
Copilot
| { | ||
| // 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) { |
There was a problem hiding this comment.
This is checked now even in the case where we just set a deadline, is that what we want?
| let deadline = &mut port_state.sink_ready_deadline; | ||
|
|
||
| // Check if we need to start the timeout | ||
| if let Some(new_contract) = new_contract { |
There was a problem hiding this comment.
It seems like we can keep new_contract as a bool and just check whether port_state.status.available_sink_contract is the same as status.available_sink_contract. The option inside an option makes it a bit confusing.
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.