search

matsstaff / stc1000p

Programmable thermostat firmware and arduino based uploader for the STC-1000 thermostat
GNU General Public License v3.0
262 stars 47 forks source link

Suspicious RANGE() function #52

Closed ArtMilburn closed 9 years ago

ArtMilburn commented 9 years ago

Hello,

I was really looking forward to using this program on my STC-1000, but unfortunately I got a version with a Holtek processor on it rather than the Microchip part.

However, in looking through the code briefly, I found a potential problem that you might want to take a look at:

static int RANGE(int x, int min, int max){ if(x>max) return min; if(x<min) return max; return x; }

If x is greater than the max, then the min is returned!? Seems like the max should be returned. Same kinda deal for x less than min.

matsstaff commented 9 years ago

Hi! Thanks for the report! It is nice to know someone gives a hoot :) However, it is really the name of the function that is misleading. It is used to limit the values that you enter with the keypad. I personally (as I think a lot of others would also), prefer it to 'roll over' when max of min is reached, instead of going all the way back in the other direction. But you are correct in that it is not really a range. I just don't really have a better name...

Cheers!

ArtMilburn commented 9 years ago

OK, I get it. It's a rollover, not a clipper.

Well, no harm, no foul.

Keep on brewing,

Art

ArtMilburn commented 9 years ago

Mats,

Just wanted to let you know that I ended up buying an STC-1000+ with dual probes from Will at Black Box Brew.

Thanks again for your hard work,

Art

On Tue, Mar 10, 2015 at 12:30 PM, matsstaff notifications@github.com wrote:

Hi! Thanks for the report! It is nice to know someone gives a hoot :) However, it is really the name of the function that is misleading. It is used to limit the values that you enter with the keypad. I personally (as I think a lot of others would also), prefer it to 'roll over' when max of min is reached, instead of going all the way back in the other direction. But you are correct in that it is not really a range. I just don't really have a better name...

Cheers!

— Reply to this email directly or view it on GitHub https://github.com/matsstaff/stc1000p/issues/52#issuecomment-78129493.