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

Simplified the sonata PWM driver #361

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

HU90m
Copy link
Collaborator

@HU90m HU90m commented Nov 27, 2024

No description provided.

There are now 6 general purpose PWM outputs and one dedicated to the
LCD's backlight. Instead of implementing the indexing logic and checks
in the driver. The driver is changed to only over map to a single PWM
output. We simply alias an array of these for the general purpose PWM
memory region.

The dutyCycle <= period assertion was removed, because dutyCycle >
period is the only way to acheive a constant high, 100% duty cycle
output from the Sonata system's PWM block.

/// A convenience structure that can map onto multiple PWM outputs.
template<size_t NumberOfPwms = 6>
struct Array
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason that this isn't a std::array? It looks like it's equivalent to a subset of std:array.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it for the compile-time bounds checking?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I originally used std::array but found a lot of std::array's methods don't work with utils::NoCopyNoMove elements.

Copy link
Contributor

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

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

This looks correct to me from a code inspection.

@@ -1,37 +1,21 @@
#pragma once
#include <array>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is stale now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yes good spot!

*/
using General = Array<6>;
/// There is one dedicated PWM for the LCD backlight.
using Lcd = Output;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe call this LcdBacklight? It's a bit confusing to have a thing called Lcd that isn't the LCD interface.

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.

3 participants