type-c-service/max_sink_voltage: Port recovery logic from v1

[RobertZ2011]

Pull in the recovery logic from v1 and add test coverage.

[Copilot]

Pull request overview

This PR ports the v1 “recovery” behavior for set_max_sink_voltage by arming the sink-ready timer as a fallback mechanism after temporarily disconnecting the sink path. It also renames the shared-state field from a “timeout” concept to an absolute “deadline” to better reflect how the timer is used. Finally, it adds an integration test that exercises the max-sink-voltage disconnect/reconnect flow and validates power-policy notifications.

Changes:

• Rename SharedState’s sink-ready tracking from sink_ready_timeout to sink_ready_deadline and update call sites/tests accordingly.
• Factor out check_sink_ready_timeout_duration(is_epr) and reuse it for both normal contract transitions and max-sink-voltage recovery.
• Add an integration test to validate that a max sink voltage change triggers a temporary consumer disconnect and subsequent recovery.

Step-by-step review guide

1. Shared-state semantics change (timeout → deadline)

   • SharedState now stores an absolute Instant (sink_ready_deadline) instead of a conceptually ambiguous “timeout”.
   • This matters because both the controller event loop and out-of-band operations (like set_max_sink_voltage) rely on the same shared timer state.

2. Centralized sink-ready timeout duration computation

   • check_sink_ready_timeout_duration(is_epr) standardizes how long the system waits before synthesizing a sink-ready event.
   • This ensures consistent behavior across “normal” PD transitions and the new max-sink-voltage recovery path.

3. Max sink voltage recovery logic

   • When changing max sink voltage while connected as a consumer, the code disables the sink path, notifies the power policy of a temporary disconnect, and arms the sink-ready deadline as a recovery mechanism.
   • Non-obvious detail: this recovery depends on the controller event receiver being able to observe the newly armed deadline even if no further hardware events occur.

4. Test coverage

   • The new test simulates: connect as sink → call set_max_sink_voltage(None) → expect ConsumerDisconnected(renegotiation=true) → wait for sink-ready recovery → expect ConsumerConnected again.
   • This is important coverage for a tricky cross-service flow (controller ↔ type-c ↔ power-policy), especially for controllers that don’t consistently emit sink-ready events.

Potential issues

#	Severity	File	Description	Code
1	🔴 High	type-c-service/src/controller/event_receiver.rs:122-126	wait_event captures sink_ready_deadline once; if another task arms the deadline while wait_event is blocked with None, the timer path won’t fire and recovery can stall indefinitely without a port event.	let timeout = self.shared_state.lock().await.sink_ready_deadline;
2	🟢 Low	type-c-service/tests/power.rs:611	Panic message is incorrect for the ConsumerDisconnected match arm, reducing debuggability when the test fails.	panic!("Did not receive provider connected event")
3	🟢 Low	type-c-service/src/controller/state.rs:6	Doc comment still says “timeout” after renaming field to sink_ready_deadline, which can confuse readers about whether this is a duration or an absolute time.	/// Sink ready timeout

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
type-c-service/tests/power.rs	Adds an integration test for max-sink-voltage disconnect + recovery; updates existing sink-ready assertions for the renamed deadline accessor.
type-c-service/src/controller/state.rs	Renames shared sink-ready tracking field/accessor to sink_ready_deadline.
type-c-service/src/controller/power.rs	Extracts check_sink_ready_timeout_duration and updates sink-ready timeout logic to use the renamed deadline field.
type-c-service/src/controller/max_sink_voltage.rs	Arms sink-ready recovery deadline when max sink voltage changes force a temporary disconnect.
type-c-service/src/controller/event_receiver.rs	Updates event receiver to use sink_ready_deadline and clear it when synthesizing sink-ready.

Comments suppressed due to low confidence (1)

type-c-service/src/controller/event_receiver.rs:126

• wait_event reads sink_ready_deadline once into a local timeout and then awaits either the port event or that captured deadline. If another task sets sink_ready_deadline while wait_event is already blocked (e.g., set_max_sink_voltage arms recovery while the event loop is waiting with None), the timer branch will never fire and recovery can stall indefinitely when no port events arrive. Consider adding an explicit wake-up/notification path when arming the deadline (e.g., a Signal/Channel in SharedState, or a dedicated loopback variant) so wait_event can re-evaluate the deadline immediately.

        let timeout = self.shared_state.lock().await.sink_ready_deadline;
        match select(self.port_event_receiver.wait_next(), async move {
            if let Some(timeout) = timeout {
                Timer::at(timeout).await;
            } else {

[jerrysxie]

Since status is the cached status, when we are switching from SPR to EPR, won't we use the timeout value for SPR instead of EPR?

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

type-c-service/tests/power.rs:676

• port0 is moved by the destructuring assignment (let TestPort { .. } = port0;), but this assertion still references port0.port. Use the local port binding instead.

                assert!(ptr::eq(psu, port0.port));

type-c-service/tests/power.rs:703

• port0 is moved by the destructuring assignment (let TestPort { .. } = port0;), but this assertion still references port0.port. Use the local port binding instead.

                assert!(ptr::eq(psu, port0.port));

type-c-service/tests/power.rs:749

• port0 is moved by the destructuring assignment (let TestPort { .. } = port0;), but this assertion still references port0.port. Use the local port binding instead.

                assert!(ptr::eq(psu, port0.port));

type-c-service/src/controller/power.rs:176

• The log message says "{}ms" but timeout is an embassy_time::Duration, so the units/format are ambiguous (and can become "msms" depending on Display). Logging the millisecond value avoids confusion.

            let timeout = Self::check_sink_ready_timeout_duration(new_status.epr);

            debug!("({}): Sink ready timeout started for {}ms", self.name, timeout);
            *deadline = Some(Instant::now() + timeout);

type-c-service/tests/power.rs:320

• Duplicate info!("Starting test: ...") line. Keeping only one avoids noisy test output and makes logs easier to scan.

        info!("Starting test: consumer flow with software sink-ready timeout");