Skip to content

power-policy-interface: Port disconnect reasons from v0.1 - #948

Open
RobertZ2011 wants to merge 4 commits into
OpenDevicePartnership:mainfrom
RobertZ2011:disconnect-flag-changes
Open

power-policy-interface: Port disconnect reasons from v0.1#948
RobertZ2011 wants to merge 4 commits into
OpenDevicePartnership:mainfrom
RobertZ2011:disconnect-flag-changes

Conversation

@RobertZ2011

@RobertZ2011 RobertZ2011 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Port changes from v0.1 that introduce an enum for disconnect reasons instead of separate flags. Also refactor existing power policy flag structs to use normal Rust structs instead of bitfield representations.

@RobertZ2011 RobertZ2011 self-assigned this Aug 21, 2026
@RobertZ2011
RobertZ2011 force-pushed the disconnect-flag-changes branch 3 times, most recently from 95095a0 to 72ce097 Compare August 24, 2026 20:30
@RobertZ2011 RobertZ2011 changed the title Disconnect flag changes power-policy-interface: Port disconnect reasons from v0.1 Aug 24, 2026
@RobertZ2011
RobertZ2011 force-pushed the disconnect-flag-changes branch 4 times, most recently from 9a747e5 to c7534ba Compare August 24, 2026 21:21
@RobertZ2011 RobertZ2011 added the BREAKING CHANGE Marks breaking changes label Aug 24, 2026
@RobertZ2011
RobertZ2011 force-pushed the disconnect-flag-changes branch from c7534ba to 60f8051 Compare August 24, 2026 21:27
@RobertZ2011
RobertZ2011 requested a lite review from Copilot August 24, 2026 21:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the power-policy interface and its consumers to replace legacy bitfield-style flags with normal Rust structs, and to model consumer disconnects using a structured DisconnectReason (wrapped in DisconnectFlags) rather than separate boolean flags. It also extends Type-C service behavior to propagate a hard-reset disconnect reason through the power-policy event path.

Changes:

  • Replace ConsumerDisconnect boolean flags with DisconnectFlags { reason: Option<DisconnectReason> } and update all notification/event plumbing accordingly.
  • Refactor ConsumerFlags/ProviderFlags from bitfield-backed types to plain Rust structs (and update tests/mocks/examples).
  • Add Type-C hard reset handling that tears down the current contract and emits a disconnect event with a reset reason (plus a new test covering it).

Reviewed changes

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

Show a summary per file
File Description
type-c-service/tests/power.rs Updates tests to use struct-based flags and validates disconnect reasons (including new hard reset coverage).
type-c-service/tests/debug_accessory.rs Updates provider capability construction to the new ProviderFlags struct.
type-c-service/src/controller/power.rs Migrates capability flags to struct fields; emits structured disconnect reasons; adds hard reset teardown path.
type-c-service/src/controller/mod.rs Routes pd_hard_reset status events into the new hard reset teardown handler.
type-c-service/src/controller/max_sink_voltage.rs Switches renegotiation disconnect signaling to DisconnectReason::ManualRenegotiation.
type-c-interface-mocks/tests/connect_disconnect.rs Updates expected capabilities to use struct-based flags.
type-c-interface-mocks/src/port/mod.rs Updates mock port state updates to use struct-based flags.
power-policy-service/tests/unconstrained.rs Updates consumer flag usage to struct defaults/fields.
power-policy-service/tests/provider.rs Updates provider flag usage to struct defaults.
power-policy-service/tests/consumer.rs Updates disconnect assertions to reason-based disconnects and refactors related test naming.
power-policy-service/tests/common/mod.rs Renames helper to assert disconnect “reason” via DisconnectFlags instead of legacy flags.
power-policy-service/src/service/mod.rs Updates notifier plumbing and disconnect processing to carry DisconnectFlags/DisconnectReason.
power-policy-service/src/service/consumer.rs Updates consumer selection/switch logic to produce a single disconnect reason and carry it through events.
power-policy-interface/src/service/notification.rs Updates service notifier trait to accept DisconnectFlags.
power-policy-interface/src/service/event.rs Updates service event payloads to use DisconnectFlags.
power-policy-interface/src/psu/notification.rs Updates PSU notifier/handler traits to accept DisconnectFlags.
power-policy-interface/src/psu/event.rs Updates PSU event payloads and notifier adapters to use DisconnectFlags.
power-policy-interface/src/charger/tests.rs Updates helper capability construction to use ConsumerFlags::default().
power-policy-interface/src/capability.rs Replaces bitfield-based flag types with plain structs and introduces DisconnectReason + DisconnectFlags.
power-policy-interface/Cargo.toml Drops bitfield/num_enum dependencies that were only needed for bitfield-based flags.
power-policy-interface-test-mocks/src/psu.rs Updates mock PSU to construct provider flags via Default and notify disconnects via DisconnectFlags.
examples/std/src/bin/power_policy.rs Updates example capability construction to struct-based consumer flags.
examples/std/Cargo.lock Removes bitfield/num_enum from the example’s resolved dependency set.
examples/rt685s-evk/Cargo.lock Removes bitfield/num_enum from the example’s resolved dependency set.
examples/pico-de-gallo/Cargo.lock Removes bitfield/num_enum from the example’s resolved dependency set.
Cargo.lock Removes bitfield/num_enum from the workspace resolved dependency set for power-policy-interface.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread type-c-service/src/controller/power.rs
Comment thread power-policy-service/src/service/consumer.rs Outdated
@RobertZ2011
RobertZ2011 force-pushed the disconnect-flag-changes branch from 60f8051 to bbee585 Compare August 24, 2026 22:08
Implement these flag structs as plain Rust structs. This reduces
complexity and the number of dependencies.
Port disconnect reasons from v0.1 branch.
@RobertZ2011
RobertZ2011 force-pushed the disconnect-flag-changes branch from bbee585 to 5587b7f Compare August 24, 2026 22:12
@RobertZ2011
RobertZ2011 marked this pull request as ready for review August 24, 2026 22:12
@RobertZ2011
RobertZ2011 requested review from a team as code owners August 24, 2026 22:13
kurtjd
kurtjd previously approved these changes Aug 25, 2026

@jerrysxie jerrysxie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixing the removal of bitfields and the porting make the PR tough to review: it was hard to separate the mechanical bitfield removal change versus the logic changes. Thankfully they were 2 different commits.

Comment thread type-c-service/src/controller/power.rs
Err(TimeoutError)
));
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked the code review agent to poke at the hard-reset recovery case a bit more and it produces 2 failing tests. Does the sequence of events in these test reflect real world sequence of events?

/// Test that a consumer can renegotiate the same contract after a PD hard reset when the
/// controller does not emit a sink ready event.
struct TestConsumerRecontractAfterHardReset;

impl Test for TestConsumerRecontractAfterHardReset {
    async fn run<'port, 'ch>(
        &mut self,
        _type_c_receiver: TypeCServiceReceiver<'port, 'ch>,
        power_policy_receiver: PowerPolicyServiceReceiver<'port, 'ch>,
        mut port0: TestPort<'port, 'ch>,
        _port1: TestPort<'port, 'ch>,
        _port2: TestPort<'port, 'ch>,
    ) {
        let connected_status = PortStatus {
            available_sink_contract: Some(POWER_CAPABILITY_5V_1A5),
            connection_state: Some(ConnectionState::Attached),
            power_role: PowerRole::Sink,
            ..Default::default()
        };
        {
            let mut mock0 = port0.mock.lock().await;
            // Queue the initial connection, hard-reset status, same-capability recontract, and
            // timer-driven sink-ready poll. The sink path is enabled for both connections.
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
            mock0.next_result_enable_sink_path.push_back(Ok(()));
            mock0.next_result_enable_sink_path.push_back(Ok(()));
        }

        // Establish the original consumer contract with an explicit sink-ready event.
        let mut port_event = PortStatusEventBitfield::none();
        port_event.set_plug_inserted_or_removed(true);
        port_event.set_new_power_contract_as_consumer(true);
        port_event.set_sink_ready(true);
        port0
            .port
            .lock()
            .await
            .process_event(Event::PortEvent(PortEvent::StatusChanged(port_event)))
            .await
            .unwrap();
        assert!(matches!(
            with_timeout(DEFAULT_PER_CALL_TIMEOUT, power_policy_receiver.receive()).await,
            Ok(PowerPolicyEvent::ConsumerConnected(_, _))
        ));

        // Tear down the active contract while the controller continues to report its capability.
        let mut port_event = PortStatusEventBitfield::none();
        port_event.set_pd_hard_reset(true);
        port0
            .port
            .lock()
            .await
            .process_event(Event::PortEvent(PortEvent::StatusChanged(port_event)))
            .await
            .unwrap();
        assert!(matches!(
            with_timeout(DEFAULT_PER_CALL_TIMEOUT, power_policy_receiver.receive()).await,
            Ok(PowerPolicyEvent::ConsumerDisconnected(
                _,
                DisconnectFlags {
                    reason: Some(DisconnectReason::Reset)
                }
            ))
        ));

        // Announce the same contract without sink ready. Recovery now depends on the software
        // deadline because some controllers do not emit a separate sink-ready event.
        let mut port_event = PortStatusEventBitfield::none();
        port_event.set_new_power_contract_as_consumer(true);
        port0
            .port
            .lock()
            .await
            .process_event(Event::PortEvent(PortEvent::StatusChanged(port_event)))
            .await
            .unwrap();

        assert!(
            port0.shared_state.lock().await.sink_ready_deadline().is_some(),
            "same-capability contract did not arm the sink ready deadline after hard reset"
        );

        // Drive the synthetic sink-ready event and verify that power policy reconnects the port.
        let Ok(event) = with_timeout(Duration::from_secs(3), port0.event_receiver.wait_event()).await else {
            panic!("sink ready deadline did not expire");
        };
        port0.port.lock().await.process_event(event).await.unwrap();

        assert!(matches!(
            with_timeout(DEFAULT_PER_CALL_TIMEOUT, power_policy_receiver.receive()).await,
            Ok(PowerPolicyEvent::ConsumerConnected(_, _))
        ));
    }
}

/// Test that a provider can renegotiate the same contract after a PD hard reset.
struct TestProviderRecontractAfterHardReset;

impl Test for TestProviderRecontractAfterHardReset {
    async fn run<'port, 'ch>(
        &mut self,
        _type_c_receiver: TypeCServiceReceiver<'port, 'ch>,
        power_policy_receiver: PowerPolicyServiceReceiver<'port, 'ch>,
        port0: TestPort<'port, 'ch>,
        _port1: TestPort<'port, 'ch>,
        _port2: TestPort<'port, 'ch>,
    ) {
        let connected_status = PortStatus {
            available_source_contract: Some(POWER_CAPABILITY_5V_1A5),
            connection_state: Some(ConnectionState::Attached),
            power_role: PowerRole::Source,
            ..Default::default()
        };
        {
            let mut mock0 = port0.mock.lock().await;
            // Queue the initial connection, hard-reset status, and same-capability recontract.
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
            mock0.next_result_get_port_status.push_back(Ok(connected_status));
        }

        // Establish the original provider contract.
        let mut port_event = PortStatusEventBitfield::none();
        port_event.set_plug_inserted_or_removed(true);
        port_event.set_new_power_contract_as_provider(true);
        port0
            .port
            .lock()
            .await
            .process_event(Event::PortEvent(PortEvent::StatusChanged(port_event)))
            .await
            .unwrap();
        assert!(matches!(
            with_timeout(DEFAULT_PER_CALL_TIMEOUT, power_policy_receiver.receive()).await,
            Ok(PowerPolicyEvent::ProviderConnected(_, _))
        ));

        // Tear down the provider while the controller continues to report its capability.
        let mut port_event = PortStatusEventBitfield::none();
        port_event.set_pd_hard_reset(true);
        port0
            .port
            .lock()
            .await
            .process_event(Event::PortEvent(PortEvent::StatusChanged(port_event)))
            .await
            .unwrap();
        assert!(matches!(
            with_timeout(DEFAULT_PER_CALL_TIMEOUT, power_policy_receiver.receive()).await,
            Ok(PowerPolicyEvent::ProviderDisconnected(_))
        ));

        // Reannounce the same capability and require it to be published as a new contract.
        let mut port_event = PortStatusEventBitfield::none();
        port_event.set_new_power_contract_as_provider(true);
        port0
            .port
            .lock()
            .await
            .process_event(Event::PortEvent(PortEvent::StatusChanged(port_event)))
            .await
            .unwrap();

        assert!(
            matches!(
                with_timeout(DEFAULT_PER_CALL_TIMEOUT, power_policy_receiver.receive()).await,
                Ok(PowerPolicyEvent::ProviderConnected(_, _))
            ),
            "same-capability provider contract was not published after hard reset"
        );
    }
}

/// Type safe wrapper for consumer disconnect flags
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[non_exhaustive]
pub enum DisconnectReason {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for clarification, the v0.1 version has a NoLongerCapable variant, and that is not needed anymore?

pub enum DisconnectReason {
    /// The device is no longer capable of providing or consuming power,
    /// no further information is available
    NoLongerCapable,
    /// The device has been physically detached
    Detached,
    /// Switching to a different PSU
    Switching,
    /// Renegotiation triggered by device
    AutoRenegotiation,
    /// Renegotiation triggered by code
    ManualRenegotiation,
    /// The device has changed its role
    RoleSwap,
    /// The device experienced a reset
    Reset,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BREAKING CHANGE Marks breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants