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

NRF52 PwmOut inverted #7707

Closed
ChiSaw opened this Issue Aug 6, 2018 · 14 comments

Comments

Projects
None yet
8 participants
@ChiSaw

ChiSaw commented Aug 6, 2018

Description

Switched from mbed-os-5.7 to mbed-os-5.9, suddenly PwmOut behaved inverted, meaning

PwmOut led(SOME_PIN);
led.write(0); // LED on

accordingly opposite cases for .write(1000);

Note: LED circuit is active-high.

Target: NRF52832
ARM_GCC 6-2017-q2-update 6.3.1 20170620 (release)
mbed-os: 5.9, mbed-cli
Steps to reproduce:
empty mbed-os project with main() containing

PwmOut led(p17);
led.write(0);
Thread::wait(3000); // to notice a change
led.write(1000);

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

@ciarmcom

This comment has been minimized.

Member

ciarmcom commented Aug 6, 2018

ARM Internal Ref: MBOTRIAGE-1495

@ciarmcom ciarmcom added the mirrored label Aug 6, 2018

@MateuszMaz

This comment has been minimized.

Contributor

MateuszMaz commented Aug 7, 2018

I'll check it.
edit:
Hey, if you are using standard NRF52832 development kit:
https://os.mbed.com/platforms/Nordic-nRF52-DK)
then according to documentation (https://bit.ly/2OkRWHO):

The LEDs are active low, meaning that writing a logical zero ('0') to the output pin will illuminate the LED.

If you are using other board, then please let me know about it.

cc @maciejbocianski @fkjagodzinski @mprse

@ChiSaw

This comment has been minimized.

ChiSaw commented Aug 8, 2018

No, I am using my own board where the NRF52832 serves as the main processor and there the LED is connected active high.

But I see where this inverted behavior comes from, someone implemented PwmOut based on the DK and applied the active-low logic from this specific hardware?

In my opinion, this should be changed to non-inverted, what do you think? One reason might be to use PWM not for LED, but for motor control, etc, where you can not always expect active-low logic :)

@jrobeson

This comment has been minimized.

Contributor

jrobeson commented Aug 8, 2018

shouldn't this sort of thing be specified at the mbed-os platform level and thus be standard across all devices? If so, then the default should be whatever they do.

@MateuszMaz

This comment has been minimized.

Contributor

MateuszMaz commented Aug 8, 2018

@ARMmbed/mbed-os-hal
the PWM waveform polarity generated by nRF-52 is inconsitient to other devices like K64F, Nucleo-F070RB, what do you think, should this behaviour be changed?

@fkjagodzinski

This comment has been minimized.

Member

fkjagodzinski commented Aug 8, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 8, 2018

the PWM waveform polarity generated by nRF-52 is inconsitient to other devices like K64F, Nucleo-F070RB, what do you think, should this behaviour be changed?

As this might not be yet specified (will be!), should follow what the rest of the boards do. They do write 0 in their init:

pwmout_write    (obj, 0);

I can't locate this for NRF52, I can see obj->percent is set to 0 but not used anywhere during init ?

The main issue here is the regression in the HAL, @MateuszMaz can you check which commit broke this?

@MateuszMaz

This comment has been minimized.

Contributor

MateuszMaz commented Aug 8, 2018

@MateuszMaz

This comment has been minimized.

Contributor

MateuszMaz commented Aug 8, 2018

@0xc0170 It looks like the problem is located in PR #6547, which is about updating Nordic NRF52 based targets to SDK 14.2

@drahnr

This comment has been minimized.

Contributor

drahnr commented Aug 8, 2018

@MateuszMaz

This comment has been minimized.

Contributor

MateuszMaz commented Aug 9, 2018

I found another problem that is connected to Pwm in target NRF-52 , I described it here #7743

@jrobeson

This comment has been minimized.

Contributor

jrobeson commented Aug 12, 2018

I covered this in #5683 I think. It was said to be fixed, but it doesn't look to be the case. I never got a chance to go back to it at the time.

MateuszMaz added a commit to MateuszMaz/mbed-os that referenced this issue Aug 13, 2018

Fix for issue ARMmbed#7707 PwmOut inverted
since obj->sequence = &obj->pulse
and most significant bit of sequence denotes the polarity, we should set it.
@MateuszMaz

This comment has been minimized.

Contributor

MateuszMaz commented Aug 13, 2018

here is pr #7779

adbridge added a commit that referenced this issue Sep 7, 2018

Fix for issue #7707 PwmOut inverted
since obj->sequence = &obj->pulse
and most significant bit of sequence denotes the polarity, we should set it.

adbridge added a commit that referenced this issue Sep 7, 2018

Fix for issue #7707 PwmOut inverted
since obj->sequence = &obj->pulse
and most significant bit of sequence denotes the polarity, we should set it.
@bmcdonnell-ionx

This comment has been minimized.

Contributor

bmcdonnell-ionx commented Sep 14, 2018

Mbed OS 5.9.7 release notes links to this issue as "fixed". Consider closing.

@ChiSaw ChiSaw closed this Sep 21, 2018

janjongboom added a commit to janjongboom/mbed-os that referenced this issue Oct 24, 2018

Fix for issue ARMmbed#7707 PwmOut inverted
since obj->sequence = &obj->pulse
and most significant bit of sequence denotes the polarity, we should set it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment