Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PCNT clock may be deactivated after migration to 0.23 #3094

Open
DamienEspitallier opened this issue Feb 4, 2025 · 1 comment
Open

PCNT clock may be deactivated after migration to 0.23 #3094

DamienEspitallier opened this issue Feb 4, 2025 · 1 comment
Labels
bug Something isn't working status:needs-attention This should be prioritized

Comments

@DamienEspitallier
Copy link

Bug description

0.23 has changed the clock enable algorithm to use GenericPeripheralGuard. But it does not work when a PCNT is initialized in a function which return only a reference to a counter as it will drop the partially moved PCNT/Unit variables

To Reproduce

here is my function which trigger the bug

fn init_counters(
        pcnt: PCNT,
        left_pin_a: Input,
        left_pin_b: Input,
        right_pin_a: Input,
        right_pin_b: Input,
    ) -> (
        pcnt::unit::Counter<'static, 0>,
        pcnt::unit::Counter<'static, 1>,
    ) {
      let pcnt = Pcnt::new(pcnt);

      let u0 = pcnt.unit0;
      u0.set_filter(Some(min(10u16 * 80, 1023u16))).unwrap();
      u0.clear();

      let ch0 = &u0.channel0;
      ch0.set_ctrl_signal(left_pin_a.peripheral_input());
      ch0.set_edge_signal(left_pin_b.peripheral_input());
      ch0.set_ctrl_mode(
          pcnt::channel::CtrlMode::Reverse,
          pcnt::channel::CtrlMode::Keep,
      );
      ch0.set_input_mode(
          pcnt::channel::EdgeMode::Decrement,
          pcnt::channel::EdgeMode::Increment,
      );

      let ch1 = &u0.channel1;
      ch1.set_ctrl_signal(left_pin_b.peripheral_input());
      ch1.set_edge_signal(left_pin_a.peripheral_input());
      ch1.set_ctrl_mode(
          pcnt::channel::CtrlMode::Reverse,
          pcnt::channel::CtrlMode::Keep,
      );
      ch1.set_input_mode(
          pcnt::channel::EdgeMode::Increment,
          pcnt::channel::EdgeMode::Decrement,
      );

      let u1 = pcnt.unit1;
      u1.set_filter(Some(min(10u16 * 80, 1023u16))).unwrap();
      u1.clear();

      let ch0 = &u1.channel0;
      ch0.set_ctrl_signal(right_pin_a.peripheral_input());
      ch0.set_edge_signal(right_pin_b.peripheral_input());
      ch0.set_ctrl_mode(
          pcnt::channel::CtrlMode::Reverse,
          pcnt::channel::CtrlMode::Keep,
      );
      ch0.set_input_mode(
          pcnt::channel::EdgeMode::Decrement,
          pcnt::channel::EdgeMode::Increment,
      );

      let ch1 = &u1.channel1;
      ch1.set_ctrl_signal(right_pin_b.peripheral_input());
      ch1.set_edge_signal(right_pin_a.peripheral_input());
      ch1.set_ctrl_mode(
          pcnt::channel::CtrlMode::Reverse,
          pcnt::channel::CtrlMode::Keep,
      );
      ch1.set_input_mode(
          pcnt::channel::EdgeMode::Increment,
          pcnt::channel::EdgeMode::Decrement,
      );

      u0.listen();
      u0.resume();
      u1.listen();
      u1.resume();

      let counter_left = u0.counter.clone();
      let counter_right = u1.counter.clone();

      (counter_left, counter_right)
  }

At the end, pcnt and units will be dropped and counters will be broken as the clock is now deactivated.

For now, I "fixed" the issue in my code by using the following hack:

let pcnt = Pcnt::new(pcnt);
let pcnt = ManuallyDrop::new(pcnt);

Expected behavior

PCNT clock should be enabled as long as a reference to pcnt, unit or counter is valid.

Environment

  • Target device: ESP32-S3
  • Crate name and version: esp-hal 0.23.1
@DamienEspitallier DamienEspitallier added bug Something isn't working status:needs-attention This should be prioritized labels Feb 4, 2025
@github-project-automation github-project-automation bot moved this to Todo in esp-rs Feb 4, 2025
@Dominaezzz
Copy link
Collaborator

AHH, interesting use case. Each object basically needs a guard field in them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status:needs-attention This should be prioritized
Projects
Status: Todo
Development

No branches or pull requests

2 participants