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.

Sign up for GitHub

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

Jump to bottom

Inconsistent Time Unit in PIDController 'period' #954

Closed
Aric5301 opened this issue Feb 24, 2018 · 3 comments
Closed

Inconsistent Time Unit in PIDController 'period' #954

Aric5301 opened this issue Feb 24, 2018 · 3 comments
Labels
good first issue Good for newcomers. type: docs Related to documentation and information.
Milestone
2019.1.1

Comments

@Aric5301
Copy link

The Time unit of the 'period' variable in PIDController is implemented as seconds in the code. (Default value is .05).
In the documentation, it does not specify which time unit is to be used when this value is set by the user, but does say that default is 50 ms.
This is confusing and not intuitive beacause from the documentation it is not clear whether the methods that set this value get a number representing miliseconds or seconds.
Whatever the correct time unit is, it should be consistent through both code and documentation.

Best Regards,
Aric Radzin,
Lead Programmer,
BumbleB #3339
@virtuald
Copy link
Member

Feel free to make a PR fixing the documentation. :)

@Aric5301
Copy link
Author

Aric5301 commented Feb 24, 2018 •
edited
Loading

I wish I could, but unfortunately I don't know to submit a PR...

@virtuald
Copy link
Member

PR = Pull Request

Some information about submitting a pull request can be found at https://help.github.com/articles/creating-a-pull-request/

@AustinShalit AustinShalit added up for grabs type: docs Related to documentation and information. labels May 8, 2018
@AustinShalit AustinShalit added this to the 2019 milestone May 8, 2018
@PeterJohnson PeterJohnson added good first issue Good for newcomers. and removed up for grabs labels Jul 30, 2018
@kasemir kasemir mentioned this issue Sep 10, 2018
Merged
pjbuterbaugh pushed a commit to pjbuterbaugh/allwpilib that referenced this issue Jun 15, 2023
@sciencewhiz
Add a Post-Installation section for WPILib Install (wpilibsuite#954)
4f3083d 
* Add a Post-Installation section for WPILib Install

macOS: drag application to launcher
Linux: enable launching of desktop icon

* fix lint

* Address review comments

Add eject step for macOS
change launcher to dock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers. type: docs Related to documentation and information.
Projects
None yet
Milestone
2019.1.1
Development

No branches or pull requests
4 participants
@PeterJohnson @virtuald @AustinShalit @Aric5301