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

enhance coding style Examples #27

Merged
merged 1 commit into from
May 10, 2015
Merged

enhance coding style Examples #27

merged 1 commit into from
May 10, 2015

Conversation

sciunto
Copy link

@sciunto sciunto commented May 9, 2015

Hi,

I made some edits of the examples, to make them easier to reuse for newcomers.

Thanks!

@orgua
Copy link

orgua commented May 10, 2015

I'm with you on this - coding style is essential in librarys, but probably this won't get merged :-(
Maintaining this lib isn't br3ttbs strong suit. maybe it's easier to fork it and release a separate version "2". I wish to hear his thoughts on this.

BTW: #defines are precompiler-commands and are no longer good practice. 10 years ago yes, but now it's better to use "static const _type =(##);" instead. Result is nearly the same, but you get much more helpful errors and warnings.

br3ttb added a commit that referenced this pull request May 10, 2015
@br3ttb
Merge pull request #27 from sciunto/examples
fb095d8 
enhance coding style Examples
@br3ttb br3ttb merged commit fb095d8 into br3ttb:master May 10, 2015
@br3ttb
Copy link
Owner

br3ttb commented May 10, 2015

This is a great addition thanks. can't believe nobody mentioned that before!

I hesitate to reply to your comment orgua, as it reads (to me anyway) more like a venting of frustration over pull request 26 than a comment on this request. But... I can see how it would be frustrating from your side, and probably owe some sort of explanation. Why did PR #27 and #24 get merged, and why have I dragged my feet on #26 (yours)? 2 reasons: one legitimate, and one stupid.

Legitimate: before release, I spent literally months perfecting and refining the working code of the library. This thing for example. I feel that if I'm going to change anything about the internals of the library, I need to make sure I do justice to that effort, and strictly vet any changes. It's hard to make time for that at this point in my life (THIS thing.) #27 #24 were high-value / peripheral changes that I completely understood. Your changes will require some education on my part to get comfortable with them.

Stupid: I'm proud of my work, and I want to be able to say that I'm solely responsible for this library. This is clearly bullshit, and something that I need to let go of. ESPECIALLY since I can't invest the time anymore to do everything myself.

So here's the deal. I'm going to go over to #26 in a couple of hours and ask you some questions to try to become satisfied with the changes you've suggested. It's quite likely that I'll get what I need, at which point I'll merge your request.

@sciunto
Copy link
Author

sciunto commented May 10, 2015

Thank you for your work @br3ttb :)

drf5n pushed a commit to drf5n/Arduino-PID-Library that referenced this pull request Mar 21, 2023
@Dlloydev
Update
91b639f 
Version 2.4.7

    Changed outputSum, outMin, outMax and error variables from int to float (issue br3ttb#27)
    Update condition (line 64) in PID_RelayOutput example (issue br3ttb#25)
    Update analogWrite.cpp and analogWrite.h to version 2.0.9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sciunto @orgua @br3ttb