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

Bournetoride patch pwm #14

Open
wants to merge 2 commits into
base: pid_branch
Choose a base branch
from

Conversation

Bournetoride
Copy link

Description: Add PWM output to Dimmer Pin

Related issue (if applicable): fixes #

Checklist:

  • [x ] The pull request is done against the latest dev branch
  • [x ] Only relevant files were touched
  • [x ] Only one feature/fix was added per PR.
  • [x ] The code change is tested and works on core 2.3.0, 2.4.2, 2.5.2, and pre-2.6
  • [x ] The code change pass travis tests. Your PR cannot be merged unless tests pass
  • [x ] I accept the CLA.

@colinl
Copy link
Owner

colinl commented Dec 27, 2020

Can I have a detailed description of what this does please?

@Bournetoride
Copy link
Author

It sends a PWM signal based on power output from PID instruction, I use it to drive a PWM fan motor. Using Dimmer command it is sent to whichever pin is defined as PWM1.

@colinl
Copy link
Owner

colinl commented Dec 30, 2020

OK, that looks reasonable. Just a couple of points.

I haven't used pwm, but I see you are scaling the pid output to the range 25 to 75, is that what is required for pwm?

In the section at the start of the file, could you comment out the line starting
#define PID_USE_PWM
That should not be the default if someone uses the file and does not specifically configure that option.
Also there is a commented out line that can be removed
//dimmer = pwm_power;

Also could you revert the change to user_config_override_sample.h please, that file is not modified on the branch.

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