Fix race condition causing lost events

[soligen2010]

If the ISR set the state of button after ret was set but before button was set to open, the event was lost

[0xPIT]

can you please clean up the PR so that I can review the actual changes? You did a lot of formatting changes, please undo them as well.

[soligen2010]

At this point no I will not undo anything. I fixed and enhanced so much stuff that I consider my fork to be the master and I have been directing everyone to my fork. If you want to take my fork in its entirety and merge it with yours I would be happy to create a pull request for everything, but I will not unwind anything I have done unless it is a bug.

I really appreciate the work you did and I really liked the concept of how your library worded (that is why I started with yours) but after a year since I forked it, the improvements and enhancement I have done are significant.

Please look at my fork. If you want it all, I will submit it all to be merged.

On Jan 15, 2017 6:46 AM, "0xPIT" notifications@github.com wrote:

can you please clean up the PR so that I can review the actual changes? You did a lot of formatting changes, please undo them as well.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/0xPIT/encoder/pull/9#issuecomment-272690037, or mute the thread https://github.com/notifications/unsubscribe-auth/APx9EIdBc_A4ae7z2Gu0XrEGaOpvPlPNks5rSgckgaJpZM4HdsdD .

[soligen2010]

You haven't replies back. Do you want to merge my whole fork? I think if you look at all the commits I did you will see that I put a lot of effort into this library and going back through all of that to undo things would be a whole lot of work that adds no value.

I think it would be a shame to not take all the fixes and enhancements I have done.

[0xPIT]

Thanks a lot, I'll try to review this on the weekend!

[soligen2010]

I think you are correct. I will do another commit to fix it

On Feb 14, 2017 1:48 PM, "Jürgen Skrotzky" notifications@github.com wrote:

@Jorgen-VikingGod commented on this pull request.

In ClickEncoder.cpp https://github.com/0xPIT/encoder/pull/9#pullrequestreview-21831400:

• if (pinA >= 0 && pinA >= 0) {

Why double check pinA? Do you mean pinA and pinB instead?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/0xPIT/encoder/pull/9#pullrequestreview-21831400, or mute the thread https://github.com/notifications/unsubscribe-auth/APx9EFKNfsC_YToh_tbfBdWIevVf3SD6ks5rcfcMgaJpZM4HdsdD .

[0xPIT]

I've manage to review most changes last night. Thanks for your contributions. I won't merge one big PR, though, but I'd like to integrate most of your changes.

Would you be willing to provide smaller PRs? Otherwise, I'll try to find time and cherry-pick changes myself.

[soligen2010]

It was never intended to be one big pull request. It started with just the first commit, but GITHUB for whatever reason just keeps adding subsequent commits onto the original old pull request. Each commit is a very reasonable scope, but don't know how to break things up so each commit is a pull request.

[soligen2010]

I think all the pull requests you have accepted are already in my fork (although perhaps done slightly differently) If you are happy with everything, then I think we can resolve all the conflicts. The only thing I can think that you might not want to include is the analog button support - putting multiple buttons (or encoder buttons) on one analog pin. It is perhaps not quite thematic with the library, but it got me out of a jam where I had no pins left. The limitation is that pressing multiple buttons at the same time wont work right (not an issue for my application)

If you want everything but this feature, I could add a commit to pull it out.