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

MCPWM 5.0 #132

Open
wants to merge 62 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
e47f8af
The beginning of a first rough draft...
usbalbin Jun 26, 2022
5a86430
Fix some of the TODOs, disable example for esp variants without mcpwm
usbalbin Jun 30, 2022
bef1ea4
Fix clippy lints
usbalbin Jul 1, 2022
208bde0
Update cfg's
usbalbin Jul 1, 2022
ad91ea1
use esp_idf_sys::*
usbalbin Jul 15, 2022
4e3b09c
Address some of the comments and add deadtime support
usbalbin Jul 19, 2022
8a1b0fd
Clippy
usbalbin Jul 19, 2022
17a3e63
Fix warnings in example
usbalbin Jul 19, 2022
a428ed6
Fix example
usbalbin Jul 20, 2022
8f2f559
Add some docs for deadtime modes
usbalbin Jul 24, 2022
d64529a
Added Mcpwm::operator_source_frequency
usbalbin Jul 25, 2022
729cb85
cargo fmt
usbalbin Jul 25, 2022
15610a2
Add more documentation
usbalbin Jul 27, 2022
cc8589d
Update example(crashing)
usbalbin Aug 5, 2022
8ca34ba
Work around float printing issue
usbalbin Aug 8, 2022
4ed2696
Clarify deadtime
usbalbin Aug 8, 2022
490a84a
Some cleanup
usbalbin Aug 8, 2022
c874e78
Fix some methods missing their self
usbalbin Aug 8, 2022
9ebbeee
Final(?) adjustments to doc
usbalbin Aug 12, 2022
d05ae44
Explain example mcpwm-simple
usbalbin Aug 12, 2022
f5a13ca
Allow setting initial duty in OperatorConfig
usbalbin Aug 31, 2022
6bc129a
Clamp timer resolution to not be greater than timer group resolution
usbalbin Aug 31, 2022
1d779db
Expose Mcpwm's unit and Operator's timer
usbalbin Aug 31, 2022
8384b06
Very rough, not working draft of MCPWM 5.0 mostly stolen from MCPWM 4.4
usbalbin Sep 16, 2022
5f01835
Refactor into different files and update example(still not compiling …
usbalbin Sep 25, 2022
5acf5fc
A bit less far from compiling...
usbalbin Oct 2, 2022
a10f4f3
Almost compiling
usbalbin Oct 9, 2022
12e34db
Compiling!!! However far from working or even testable...
usbalbin Oct 11, 2022
0caf311
...just kidding. However now it is compiling!! But now quite running
usbalbin Oct 13, 2022
451dcd9
Add TODO comment
usbalbin Oct 13, 2022
952a36f
fmt
usbalbin Oct 13, 2022
9ec6458
Fix some, far from all, warnings
usbalbin Oct 13, 2022
ca5685e
Unchecked stuff
usbalbin Nov 14, 2022
dc05884
Compiling again after refactor, still far from working
usbalbin Nov 27, 2022
d932af4
Do all configuration before building the real types(not compiling)
usbalbin Nov 27, 2022
8d80760
Compiling again, still some work left to do before working
usbalbin Nov 28, 2022
1907eef
Clean up, fix some todos, might be testable now/soon!
usbalbin Nov 28, 2022
bb8d957
Fix some more warnings, and some cleanup
usbalbin Nov 28, 2022
e7e755e
Remove deadtime example
usbalbin Nov 28, 2022
d20ae62
Fix example and more cleanup
usbalbin Nov 28, 2022
7b2a929
Dead time should probably wait...
usbalbin Nov 28, 2022
6e8d31e
Operators constructor does not need to be pub, Timer::resolution is a…
usbalbin Nov 28, 2022
d3e44c1
Some cleanup, added comments and removed temp changes in .cargo
usbalbin Dec 3, 2022
c15c69f
fmt
usbalbin Dec 3, 2022
f053be1
Remove some dead code, fix clippy warnings and silent those that cant…
usbalbin Dec 3, 2022
326299f
Update cfgs to require IDF 5.0 for MCPWM (for now)
usbalbin Dec 3, 2022
44cd1d8
fmt
usbalbin Dec 3, 2022
5c4738b
Put example behind cfg
usbalbin Dec 3, 2022
7ab9357
Add extern functions for vararg
usbalbin Dec 6, 2022
aa19703
One large variadic call instead of multiple small
usbalbin Dec 6, 2022
55745e6
Avoid std
usbalbin Dec 10, 2022
1c3dae4
fmt
usbalbin Dec 10, 2022
8bca808
Fix typo
usbalbin Dec 10, 2022
7063f05
Fixes after testing on real hardware (ESP32-S3). Flags set to match I…
usbalbin Dec 10, 2022
1bd2dfd
Remove leftover debug code
usbalbin Dec 10, 2022
d786f05
Remove patch
usbalbin Dec 21, 2022
4acbca7
Resolve some of the comments
usbalbin Jan 25, 2023
6913374
Make both comparators obligatory
usbalbin Jan 28, 2023
cc76cb5
Make Generators mandatory
usbalbin Feb 6, 2023
bdcb5e4
Update example
usbalbin Feb 6, 2023
316f685
Remove some unwraps
usbalbin Feb 6, 2023
66d0dde
fmt
usbalbin Feb 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions examples/mcpwm-simple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/// Simple example showing how MCPWM may be used to generate two synchronized pwm signals with varying duty.
/// The duty on the pin4 will increase from 0% all the way up to 100% and repeat. At the same time the duty
/// on pin 5 will go from 100% down to 0%
///
/// # duty = 10%
///
/// . .
/// . .
/// .------. .------.
/// | | | |
/// pin4 | | | |
/// | | | |
/// ----- -------------------------- --------------------------
/// . .
/// . .
/// .------------------------. .------------------------.
/// | | | |
/// pin5 | | | |
/// | | | |
/// ----- -------- --------
/// . .
///
///
/// # duty = 50%
/// . .
/// . .
/// .---------------. .---------------.
/// | | | |
/// pin4 | | | |
/// | | | |
/// ----- ----------------- -----------------
/// . .
/// . .
/// .---------------. .---------------.
/// | | | |
/// pin5 | | | |
/// | | | |
/// ----- ----------------- -----------------
/// . .
///
///
/// # duty = 90%
/// . .
/// . .
/// .------------------------. .------------------------.
/// | | | |
/// pin4 | | | |
/// | | | |
/// ----- -------- --------
/// . .
/// . .
/// .------. .------.
/// | | | |
/// pin5 | | | |
/// | | | |
/// ----- -------------------------- --------------------------
/// . .

#[cfg(all(any(esp32, esp32s3), esp_idf_version_major = "5"))]
fn main() -> anyhow::Result<()> {
use embedded_hal::delay::DelayUs;

use esp_idf_hal::delay::FreeRtos;
use esp_idf_hal::mcpwm::{OperatorConfig, Timer, TimerConfig};
use esp_idf_hal::prelude::Peripherals;

esp_idf_sys::link_patches();

println!("Configuring MCPWM");

let peripherals = Peripherals::take().unwrap();
let timer_config = TimerConfig::default().period_ticks(8_000); // 10kHz
let operator_config = OperatorConfig::default(peripherals.pins.gpio4, peripherals.pins.gpio5);
let timer = Timer::new(peripherals.mcpwm0.timer0, timer_config);

let mut timer = timer
.into_connection()
.attatch_operator0(peripherals.mcpwm0.operator0, operator_config);

// Borrow references to the contained timer and operator
let (timer, operator, _, _) = timer.split();

println!("Starting duty-cycle loop");

let period_ticks = timer.get_period_peak();

// TODO: Will this work as expected in UP_DOWN counter mode?
for duty in (0..period_ticks).step_by(10).cycle() {
if duty % 100 == 0 {
println!(
"cmp: {}, duty {}%",
duty,
100 * u32::from(duty) / u32::from(period_ticks)
);
}

operator.set_compare_value_x(duty)?; // In this configuration this controls the duty on pin4
operator.set_compare_value_y(period_ticks - duty)?; // and this controls pin5
FreeRtos.delay_ms(10)?;
}

unreachable!()
}

#[cfg(not(all(any(esp32, esp32s3), esp_idf_version_major = "5")))]
fn main() {
esp_idf_sys::link_patches();

println!("Sorry MCPWM is only supported on ESP32 and ESP32-S3");
}
6 changes: 6 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ pub mod ledc;
not(feature = "riscv-ulp-hal")
))]
pub mod mac;
#[cfg(all(
any(esp32, esp32s3),
esp_idf_version_major = "5",
not(feature = "riscv-ulp-hal")
))]
pub mod mcpwm;
#[cfg(not(feature = "riscv-ulp-hal"))]
pub mod modem;
pub mod peripheral;
Expand Down
127 changes: 127 additions & 0 deletions src/mcpwm/comparator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
use core::ptr;

use esp_idf_sys::{
esp, mcpwm_cmpr_handle_t, mcpwm_comparator_config_t, mcpwm_comparator_config_t__bindgen_ty_1,
mcpwm_gen_compare_event_action_t, mcpwm_gen_handle_t, mcpwm_gen_t, mcpwm_new_comparator,
mcpwm_oper_handle_t, mcpwm_timer_direction_t_MCPWM_TIMER_DIRECTION_DOWN,
mcpwm_timer_direction_t_MCPWM_TIMER_DIRECTION_UP,
};

use super::generator::{CountingDirection, NoCmpMatchConfig, OnMatchCfg};

trait ComparatorChannel {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The implementation uses lot's of traits which results in a heavy utilization of generics, making the code difficult to comprehend and probably difficult to use.

I don't mind generics and traits, if they cannot be avoided, but I'm questioning the utility of almost all traits in this module. Most of them can be replaced with alternatives: enums or simple options.

I'll comment on each trait below.

For ComparatorChannel specifically: why not just

enum ComparatorChannel {
    ChannelX,
    ChannelY,
}


pub struct CmpX;
impl ComparatorChannel for CmpX {}

pub struct CmpY;
impl ComparatorChannel for CmpY {}

pub trait OptionalCmp {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am struggling to understand what OptionalCmp is giving me that a simple Option<&'a mut Comparator> won't?

Struct Comparator can then just have a fn counting_direction(&self) -> CountingDirection method.

Would you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I am not quite happy with the huge amount of traits either :/

The motivation behind OptionalCmp is to only provide a operator.set_compare_value_x iff CMPX is a Comparator.

impl<const N: u8, G, GENA, GENB, CMPY> Operator<N, G, Comparator, CMPY, GENA, GENB>

I personally do not quite like a method doing that check at runtime. I will hower change it if you would.

Copy link
Contributor Author

@usbalbin usbalbin Jan 25, 2023

Choose a reason for hiding this comment

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

I do not have time the time to check right now but I believe, if I am not mistaken, there was also something similar when building the generator config. The generator config should not depend on events from comparators that are not provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought, perhaps both comparators should be obligatory since they can not be used for anything else outside their operator anyways...

type OnMatchCfg: OnMatchCfg;
type Cfg: OptionalCmpCfg<Cmp = Self>;

fn get_comparator_mut(&mut self) -> Option<&mut Comparator>;
}

impl OptionalCmp for Comparator {
type OnMatchCfg = super::generator::CountingDirection;
type Cfg = ComparatorConfig;

fn get_comparator_mut(&mut self) -> Option<&mut Comparator> {
Some(self)
}
}

pub struct NoCmp;
impl OptionalCmp for NoCmp {
type OnMatchCfg = super::generator::NoCmpMatchConfig;
type Cfg = NoCmpCfg;

fn get_comparator_mut(&mut self) -> Option<&mut Comparator> {
None
}
}

pub struct Comparator(pub(crate) mcpwm_cmpr_handle_t);

impl Comparator {
pub(crate) unsafe fn configure(&mut self, gen: &mut mcpwm_gen_t, cfg: CountingDirection) {
extern "C" {
fn mcpwm_generator_set_actions_on_compare_event(
gen: mcpwm_gen_handle_t,
ev_act0: mcpwm_gen_compare_event_action_t,
ev_act1: mcpwm_gen_compare_event_action_t,
ev_act_end: mcpwm_gen_compare_event_action_t,
) -> esp_idf_sys::esp_err_t;
}

esp!(mcpwm_generator_set_actions_on_compare_event(
gen,
mcpwm_gen_compare_event_action_t {
direction: mcpwm_timer_direction_t_MCPWM_TIMER_DIRECTION_UP,
comparator: self.0,
action: cfg.counting_up.into(),
},
mcpwm_gen_compare_event_action_t {
direction: mcpwm_timer_direction_t_MCPWM_TIMER_DIRECTION_DOWN,
comparator: self.0,
action: cfg.counting_down.into(),
},
mcpwm_gen_compare_event_action_t {
// <-- This marks the last argument in the variadic list
comparator: ptr::null_mut(),
..Default::default()
}
))
.unwrap();
}
}

#[derive(Debug, Clone, Copy)]
pub struct ComparatorConfig {
flags: mcpwm_comparator_config_t__bindgen_ty_1,
}

impl Default for ComparatorConfig {
fn default() -> Self {
let mut flags: mcpwm_comparator_config_t__bindgen_ty_1 = Default::default();
// TODO: What should be set here?
flags.set_update_cmp_on_tep(0);
flags.set_update_cmp_on_tez(1);
Comment on lines +75 to +76
Copy link
Contributor Author

@usbalbin usbalbin Dec 22, 2022

Choose a reason for hiding this comment

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

Just remembered espressif/esp-idf#9904 . Do you think tep should also be set to 1 to avoid that? I have not tested this PR for that problem yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
flags.set_update_cmp_on_tep(0);
flags.set_update_cmp_on_tez(1);
flags.set_update_cmp_on_tep(1);
flags.set_update_cmp_on_tez(1);

flags.set_update_cmp_on_sync(0);
Self { flags }
}
}

pub struct NoCmpCfg;

pub trait OptionalCmpCfg {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto here. Why are we hiding simple Options behind traits with just two implementations: none and an actual one?

type OnMatchCfg: OnMatchCfg;
type Cmp: OptionalCmp;
unsafe fn init(self, operator_handle: mcpwm_oper_handle_t) -> Self::Cmp;
}

impl OptionalCmpCfg for NoCmpCfg {
type OnMatchCfg = NoCmpMatchConfig;
type Cmp = NoCmp;

unsafe fn init(self, _operator_handle: mcpwm_oper_handle_t) -> NoCmp {
NoCmp
}
}
impl OptionalCmpCfg for ComparatorConfig {
type OnMatchCfg = CountingDirection;
type Cmp = Comparator;

unsafe fn init(self, operator_handle: mcpwm_oper_handle_t) -> Comparator {
let cfg = mcpwm_comparator_config_t { flags: self.flags };

let mut cmp = ptr::null_mut();
unsafe {
esp!(mcpwm_new_comparator(operator_handle, &cfg, &mut cmp)).unwrap();
}

Comparator(cmp)
}
}
Loading