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

Info: Custom PWM routines not neccessary for latest stage core 2.5.0 #4917

Closed
Jason2866 opened this issue Jan 13, 2019 · 17 comments
Closed

Info: Custom PWM routines not neccessary for latest stage core 2.5.0 #4917

Jason2866 opened this issue Jan 13, 2019 · 17 comments
Labels
enhancement Type - Enhancement that will be worked on

Comments

@Jason2866
Copy link
Collaborator

Have you look for this feature in other issues and in the wiki?
yes
Is your feature request related to a problem? Please describe.
Core 2.5.0 stage solved the pwm issues. No need anymore for self written code
Describe the solution you'd like
Delete / disable core_esp8266_timer.c, core_esp8266_wiring_digital.c and core_esp8266_wiring_pwm.c for core 2.5.0 feature/stage
Describe alternatives you've considered
None
Additional context
esp8266/Arduino#5578

(Please, remember to close the issue when the problem has been addressed)

@arendst
Copy link
Owner

arendst commented Jan 13, 2019

How did you test?

I tried both enabling the defines and removing the three files but in both cases the PWM channels are dead. No output on an Arilux LC01 three color PWM device...

@Jason2866
Copy link
Collaborator Author

On a wemos mini on GPIO 2 (connected on board led) with PWM1i

@arendst
Copy link
Owner

arendst commented Jan 13, 2019

Ah. now tried the official Staged version. This works as expected.

I first used your version but that doesn't seem to work.

@ascillato2 ascillato2 added the enhancement Type - Enhancement that will be worked on label Jan 13, 2019
@Jason2866
Copy link
Collaborator Author

Still on beta2 on my repo. Should i go to this version? The commits made since beta2 are great steps in the right direction.

@arendst
Copy link
Owner

arendst commented Jan 13, 2019

Ah that makes sense.

No stay at Beta 2 as that works for all and Arduino also.

Let's wait for the Core team to release a new version before updating yours

@arendst arendst added the on hold by dev team Result - Feature request put on hold by member of development team label Jan 13, 2019
@ascillato
Copy link
Contributor

Core 2.5.0 beta 3 has been released :)

@Jason2866
Copy link
Collaborator Author

Jason2866 commented Jan 26, 2019

PR for Platformio.ini to work with my repo which is now on Beta3
Updated board and platforms.txt for Arduino users
Edit:
All needed changes are done to use Beta3

@arendst
Copy link
Owner

arendst commented Jan 26, 2019

@Jason2866 I keep getting major file size differences for sonoff.bin using your platformio beta3 repository (571768) and the current stage version (557472). I would expect them to be close considering the time between beta3 release and the current stage version.

The stage version size also comes close to the Arduino IDE beta3 version as released by @andrethomas.

I expect there is something fishy with your beta3 platformio repository...

@Jason2866
Copy link
Collaborator Author

@arendst Thx for info. Will investigate.

@Jason2866
Copy link
Collaborator Author

Jason2866 commented Jan 26, 2019

@arendst
Just build a standard sonoff bin. Size is a little bit smaller as the build on thehackbox from Andre.
As always platformio builds are a bit smaller... Everything seems okay.
Have you deleted in folder .platformio in folders platforms and packages the folders containing 8266 with long name src.....

@Jason2866
Copy link
Collaborator Author

My setup: Windows 10 pro 64bit with all updates. IDE VSC with platformio plugin

@arendst
Copy link
Owner

arendst commented Jan 26, 2019

I'm out. Will try tomorrow.

@arendst
Copy link
Owner

arendst commented Jan 27, 2019

Tested it and found the cause of my problem: I didn't do the pio update so was still using the old beta2 while in platformio.ini the option nox was removed. This resulted in the larger binary as additional exception code was compiled.

So it is fixed after the pio update command.

Next action is finding a secure way to disable the 2.4.0 PWM code as I seem to have problems with the defines when used with different core version; sometimes only two out of three files are undefined, other moments all three files seem to be undefined...

To be continued...

@arendst
Copy link
Owner

arendst commented Jan 27, 2019

Still testing.

@Jason2866 would you mind to add the following lines to your 2.5.0_beta3 for tasmota repository. It allows Tasmota to show the correct "released" beta version. Pls add these lines to file cores/esp8266/core_version.h:

#define ARDUINO_ESP8266_RELEASE_2_5_0_BETA3
#define ARDUINO_ESP8266_RELEASE "2_5_0_BETA3"

@arendst
Copy link
Owner

arendst commented Jan 27, 2019

Testing finished.

As of 2.5.0-beta3 the custom PWM files are no longer needed.

They have defines enabled to act accordingly depending on core version.

@arendst arendst removed the on hold by dev team Result - Feature request put on hold by member of development team label Jan 27, 2019
@Jason2866
Copy link
Collaborator Author

If you do apio update you will see:

image

@arendst
Copy link
Owner

arendst commented Jan 27, 2019

Great! Verified. Works!

arendst added a commit that referenced this issue Jan 27, 2019
 * Add core version conditional compile options to provided PWM files (#4917)
 * Add support for inverted buttons and inverted buttons without pullup (#4914)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type - Enhancement that will be worked on
Projects
None yet
Development

No branches or pull requests

4 participants