search

rlogiacco / AnalogButtons

Arduino library to wire multiple buttons to one single analog pin
GNU Lesser General Public License v3.0
54 stars 13 forks source link

Fix for Button ADC value of Zero #6

Closed fraser125 closed 7 years ago

fraser125 commented 7 years ago

This addresses an issue that I was having when the Button ADC value was Zero.

rlogiacco commented 7 years ago

So those changes are related to #5, but rather than bumping the data type to unsigned I would prefer to improve the entering condition of the if statement, so that it would not break on a value of 0.

Would something like the following do?

if ((reading >= buttons[i].value - margin || reading <= margin) && reading <= buttons[i].value + margin) {
fraser125 commented 7 years ago

I agree that the reading can never be less than zero. The reason this "makes sense", I'm still not saying it's the right solution. The if statement calculates "value - margin" since the result of this calculation doesn't have it's own variable type the compiler makes an assumption based on the other variables in the 'if' statement, in order to allow the result of this calculation to be negative the other values have to support negative numbers.

For example the following achieves the same thing as my proposed fix for this edge case. I'm not thrilled with the amount of casting that happens in this line but the results are correct based on my testing, it also shows where the actual issue occurs vs. being in far off places in the code where it's not clear what the actual use case is. if ((int)reading >= (int)buttons[i].value - margin && (int)reading <= (int)buttons[i].value + margin) {

I did test your proposed 'if' statement and it fired on the wrong button. The relevant test data is if ((reading >= buttons[i].value - margin || reading <= margin) && reading <= buttons[i].value + margin) {

reading = 0 buttons[i].value - margin =629 margin = 10 buttons[i].value + margin = 649 Inside If Select click

It should of been Right click, Select is the first Button that is added to the AnalogButton object. Commenting out the Add for the Select button causes the Left button to fire (next one added).

if ((reading >= buttons[i].value - margin || reading <= margin) && reading <= buttons[i].value + margin) {

reading = 0 buttons[i].value - margin =398 margin = 10 buttons[i].value + margin = 418 Inside If Left click

At this time I would like to cancel this pull request and create a new one based on the 'if' statement above with the casting. It actually works better than my original proposal and is much more clear on where the issue is.