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

Feature/pwm freq #350

Merged
merged 2 commits into from
Jun 24, 2022
Merged

Feature/pwm freq #350

merged 2 commits into from
Jun 24, 2022

Conversation

ctacke
Copy link
Contributor

@ctacke ctacke commented Jun 21, 2022

No description provided.

@adrianstevens
Copy link
Contributor

Retargeting to develop

@adrianstevens adrianstevens changed the base branch from main to develop June 21, 2022 21:30
@@ -168,9 +168,9 @@ public bool IsOn
Voltage blueLedForwardVoltage,
CommonType commonType = CommonType.CommonCathode) :
this(
device.CreatePwmPort(redPwmPin),
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of defining DefaultFrequency in every peripheral driver could we just create a CreatePwmPort overload that just takes a single param?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see you have different values for each peripheral .... that makes sense

@@ -41,11 +53,11 @@ public PiezoSpeaker(IPwmPort port)
/// </summary>
/// <param name="frequency">The frequency in hertz of the tone to be played</param>
/// <param name="duration">How long the note is played in milliseconds, if durration is 0, tone plays indefinitely</param>
public async Task PlayTone(float frequency, int duration = 0)
public async Task PlayTone(Frequency frequency, int duration = 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should unitize PiezoSpeaker as well - I'll file an issue :)

@ctacke ctacke merged commit 517ef16 into develop Jun 24, 2022
@jorgedevs jorgedevs deleted the feature/pwm-freq branch November 6, 2022 18:50
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