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

Initial version of code to support multiple hardware timers #2497

Merged
merged 3 commits into from
Feb 16, 2019

Conversation

pjsg
Copy link
Member

@pjsg pjsg commented Sep 23, 2018

Fixes #2494.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.

This adds support to the hw_timer.c file to be able to have multiple timers running at the same time (multiplexed in software). This code is just about working and could do with some cleanup. It also may crash, burn or cause other problems. However, it does run gpio.pulse and pwm at the same time (in a simple case) and appear to work.

@TerryE
Copy link
Collaborator

TerryE commented Sep 23, 2018

Philip, this one needs some careful review. I will try to do this in the next couple of days

@pjsg
Copy link
Member Author

pjsg commented Sep 23, 2018 via email

@@ -10,7 +10,7 @@
#include "pin_map.h"
#include "driver/gpio16.h"

#define TIMER_OWNER 'P'
#define TIMER_OWNER (('P' << 8) + 'U')
Copy link
Member Author

Choose a reason for hiding this comment

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

This was one of the problems -- it used the same value as the PWM module.

@@ -434,7 +434,7 @@ static int gpio_pulse_start(lua_State *L) {
pulser->next_adjust = initial_adjust;

// Now start things up
if (!platform_hw_timer_init(TIMER_OWNER, FRC1_SOURCE, TRUE)) {
if (!platform_hw_timer_init(TIMER_OWNER, FRC1_SOURCE, FALSE)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why I used autoload=true when I first wrote the code. It wasn't needed. I did make the new hw_timer stuff work with autoload=true before I switched this over to FALSE.

@pjsg
Copy link
Member Author

pjsg commented Sep 24, 2018

I also added some more comments to the code....

@marcelstoer marcelstoer added this to the Next Release milestone Oct 16, 2018
@jmd13391
Copy link

jmd13391 commented Nov 5, 2018

What is the dev & master promotion schedule for this new feature?

@marcelstoer
Copy link
Member

If you click the milestone in the right column (name "Next release") you'll see that the due date is early December. That is the planned dev->master snap. Precondition is that @TerryE or some other committer approves this and merges it to dev until mid November or so. However, we're usually quite flexible with our self-imposed obligations 😉

See https://nodemcu.readthedocs.io/en/latest/#releases for more information on this process.

@TerryE
Copy link
Collaborator

TerryE commented Nov 6, 2018

Yup. Nag received. Sorry.

@marcelstoer marcelstoer removed this from the Next Release milestone Nov 22, 2018
@marcelstoer marcelstoer added this to the Next release milestone Dec 7, 2018
@jmd13391
Copy link

jmd13391 commented Jan 8, 2019

What is preventing this from being merged?

@TerryE
Copy link
Collaborator

TerryE commented Jan 8, 2019

What is preventing this from being merged?

Precondition is that @TerryE or some other committer approves this and merges it to dev.

Just time and effort to review and test this, sorry.

@etfloyd
Copy link

etfloyd commented Jan 21, 2019

Adding another voice to encourage moving forward with this. It's essential to what I'm doing (need both gpio.pulse and pwm) so I'm having to maintain a separate local dev branch so I can merge it. No problems with it so far, FWIW.

@marcelstoer marcelstoer merged commit d758304 into nodemcu:dev Feb 16, 2019
@jmd13391 jmd13391 mentioned this pull request May 13, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants