Skip to content

Conversation

@CosminGGeorgescu
Copy link

@CosminGGeorgescu CosminGGeorgescu commented Nov 4, 2022

Pull Request Overview

This pull request adds partial Led Pwm support for the esp32-c3

Testing Strategy

This pull request was tested by me :)

TODO or Help Wanted

This pull request still needs Pwm generators implementation

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

Copy link

@alexandruradovici alexandruradovici left a comment

Choose a reason for hiding this comment

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

The next step is to implement the Pwm related trait.


//functions regarding interrupts

pub fn handle_interrupt(&self) {

Choose a reason for hiding this comment

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

Do you need to handle interrupts? What is the purpose of interrupts?

Copy link
Author

Choose a reason for hiding this comment

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

The same questions I've asked myself and haven't found answers yet. Thought they made the interrupt for a reason and it is imperative to handle them

Choose a reason for hiding this comment

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

Please document that and post a link to the documentation text.

Copy link
Author

Choose a reason for hiding this comment

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

@alexandruradovici
Copy link

Any updates here?

@CosminGGeorgescu
Copy link
Author

The LED PWM clock frequency is based off the clock source used for the CPU and/or the CPU clock frequency. Should the "get_maximum_frequency_hz()" function from the Pwm hil return the maximum frequency achievable with any clock source or with the in-use clock source ?

@alexandruradovici
Copy link

This needs to be the max freq that the PWM outputs.

interrupts::IRQ_LEDC => {
//handler is unimplemented yet
//not sure exactly what to do :)
self.led_pwm.handle_interrupt();

Choose a reason for hiding this comment

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

What is the purpose of the PWM interrupt?

Copy link
Author

Choose a reason for hiding this comment

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

It is triggered either when a timer counter overflows for a set number of times, when a fade has finished or when a timer counter has reached it's max value.

Comment on lines 40 to 41
//x used for timers, value between [0, 3]
//n used for pwm generators, value between [0, 5]

Choose a reason for hiding this comment

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

What is x?

Copy link
Author

Choose a reason for hiding this comment

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

https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf#page=697&zoom=200,0,470

The following sections refer to the timers collectively as Timerx (where x ranges from 0 to 3). Likewise, the six PWM generators are also identical in features and operation, and thus are collectively referred to as PWMn (where n ranges from 0 to 5).

as seen in the official documentation

@alexandruradovici
Copy link

The LED PWM clock frequency is based off the clock source used for the CPU and/or the CPU clock frequency. Should the "get_maximum_frequency_hz()" function from the Pwm hil return the maximum frequency achievable with any clock source or with the in-use clock source ?

If it is a method, than the in-use clock source, if it is an associated function, than for any clock source (probably you have to receive the clock as a parameter).

@CosminGGeorgescu CosminGGeorgescu marked this pull request as draft August 1, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants