Skip to content

Conversation

@soburi
Copy link
Member

@soburi soburi commented Sep 23, 2022

The 'pwms' node under the 'zephyr,user' defines available PWM channels.

The 'pwm-pins' defines the mapping of PWM channels to pin numbers.
API looks up the 'pwm-pins' when querying the PWM channel by digital pin number.
So it should be in the same order as 'pwms' node.

PWM channels should configure and tied to the output pins.
The period property of the PWM channel is used as resolution as PWM.
For original Arduino API compatibility, it should be 255.
The period property and API argument determines the duty ratio.


I verified this PR with mkrzero and nano_33_ble.
Some pins of the mkrzero seem not to work correctly.
Only verified configuration only enableed.

@@ -0,0 +1,4 @@
CONFIG_CPLUSPLUS=y
CONFIG_ARDUINO_API=y
CONFIG_GPIO=y
Copy link
Member

Choose a reason for hiding this comment

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

Same here, gpio and CPP seem redundant

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed it.

*/

#include <Arduino.h>
#include "zephyrSerial.h"
Copy link
Member

Choose a reason for hiding this comment

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

including zephyrSerial shouldn't really be necessary as it is included in Arduino.h itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed it.

pinctrl-0 = <&pwm1_default>;
pinctrl-1 = <&pwm1_sleep>;
pinctrl-names = "default", "sleep";
};
Copy link
Member

Choose a reason for hiding this comment

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

Having documentation as to what all needs to be added in overlays might help here as well.

The 'pwms' node under the 'zephyr,user' defines available PWM channels.

The 'pwm-pins' defines the mapping of PWM channels to pin numbers.
API looks up the 'pwm-pins' when querying the PWM channel by digital
pin number. So it should be in the same order as 'pwms' node.

PWM channels should configure and tied to the output pins.
The period property of the PWM channel is used as resolution as PWM.
For original Arduino API compatibility, it should be 255.
The period property and API argument determines the duty ratio.

Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@fujitsu.com>
Copy link
Member

@DhruvaG2000 DhruvaG2000 left a comment

Choose a reason for hiding this comment

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

Built locally, and it does build.
Also flashed it on my nano 33 ble sense, and changed the sample a bit to rather use LED_BUILTIN.
LGTM.
Minor nitpicks, but otherwise can merge.

@DhruvaG2000
Copy link
Member

DhruvaG2000 commented Sep 25, 2022

I thought I would have rebase access, but seems like I can't squash commits, please can you squash my last commit in previous one?
Also rebase on top of main please.
Thank you

Added a fade sample to demonstrate how to use analogWrite API

Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@fujitsu.com>
@soburi
Copy link
Member Author

soburi commented Sep 25, 2022

I thought I would have rebase access, but seems like I can't squash commits, please can you squash my last commit in previous one? Also rebase on top of main please. Thank you

squash and update done.

Copy link
Member

@DhruvaG2000 DhruvaG2000 left a comment

Choose a reason for hiding this comment

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

LGTM!

@DhruvaG2000 DhruvaG2000 merged commit 37f6176 into zephyrproject-rtos:main Sep 25, 2022
@soburi soburi deleted the pwm branch September 25, 2022 16:10
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