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

Fix/pwm new #1433

Merged
merged 4 commits into from
Aug 28, 2018
Merged

Fix/pwm new #1433

merged 4 commits into from
Aug 28, 2018

Conversation

perpernorbi
Copy link
Contributor

HardwarePWM was not working, probably since custom pwm implementation has been set to default. The Basic_HwPWM sample is not working. All GPIO pins are at Vcc, the voltage on GPIO2 does not change.

The custom pwm implementation contains a bug. Converting duties and periods is malfunctioning. See StefanBruens/ESP8266_new_pwm#24 for details.

These changes correct the conversion in pwm.c (using .patches/pwm.patch). Also, minor refactoring has been done in samples/Basic_HwPWM.

To test the changes, build and deploy Basic_HwPWM, and verify voltages using a multimeter. Voltages should approximately be the same, regardless ENABLE_CUSTOM_PWM is set to 0 or 1.

Conversions in pwm.c are bugous. Fix is already available for the
original repository, but the merge request has not yet been accepted.
See StefanBruens/ESP8266_new_pwm#26
for details.
@perpernorbi perpernorbi mentioned this pull request Aug 24, 2018
@slaff
Copy link
Contributor

slaff commented Aug 26, 2018

@perpernorbi Please, fix the coding style

!!! samples/Basic_HwPWM/app/application.cpp not compliant to coding style, here is the fix:
--- samples/Basic_HwPWM/app/application.cpp	2018-08-24 16:08:23.471101371 +0000
+++ /dev/fd/63	2018-08-24 16:08:29.427120477 +0000
@@ -27,7 +27,7 @@
 bool countUp = true;
 
 int maxDuty = HW_pwm.getMaxDuty();
-int32 inc = maxDuty/50;
+int32 inc = maxDuty / 50;
 
 void doPWM()
 {
@@ -58,12 +58,12 @@
 
 	// Setting PWM values on 8 different pins
 	HW_pwm.analogWrite(4, maxDuty);
-	HW_pwm.analogWrite(5, maxDuty/2);
+	HW_pwm.analogWrite(5, maxDuty / 2);
 	HW_pwm.analogWrite(0, maxDuty);
-	HW_pwm.analogWrite(2, maxDuty/2);
+	HW_pwm.analogWrite(2, maxDuty / 2);
 	HW_pwm.analogWrite(15, 0);
-	HW_pwm.analogWrite(13, maxDuty/3);
-	HW_pwm.analogWrite(12, 2*maxDuty/3);
+	HW_pwm.analogWrite(13, maxDuty / 3);
+	HW_pwm.analogWrite(12, 2 * maxDuty / 3);
 	HW_pwm.analogWrite(14, maxDuty);
 
 	debugf("PWM output set on all 8 Pins. Kindly check...");

Make sure to read the release notes related to coding style:
https://github.com/SmingHub/Sming/releases/tag/3.6.1

@slaff
Copy link
Contributor

slaff commented Aug 26, 2018

@MayaPosch can you check if that PR is working for you?

Class HardwarePWM actually provides a function to query the maximum
value of duty.
ENABLE_CUSTOM_PWM defaults to 1 since d870c27.
@MayaPosch
Copy link
Contributor

MayaPosch commented Aug 26, 2018

@slaff I'll let you know tomorrow (Monday). Need to set up the infrastructure first before I can run a test :)

@MayaPosch
Copy link
Contributor

@slaff Just tested the PR with my setup. Seems to have fixed the PWM issues. Also finally got the fine-grained control over the output now, with up to 0x6F (111) steps tested from 0 to nearly 3.3V with a DMM directly measuring the pin (GPIO14) in DC voltage mode.

@slaff slaff merged commit 0b646a5 into SmingHub:develop Aug 28, 2018
mikee47 added a commit to mikee47/Sming that referenced this pull request Sep 11, 2018
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