search

adafruit / Adafruit_CircuitPython_SCD4X

CircuitPython/Python driver for Sensirion SCD40 & SCD41
MIT License
19 stars 10 forks source link

Subtract demominator by one #20

Closed kolcz closed 6 days ago

kolcz commented 4 months ago

Add subtraction by 1 to denominator

bablokb commented 1 week ago

Is there a reason why this is not merged? From a practical point of view, it does not matter (one per-mill of a degree), but there is no reason to have it wrong. The only argument would be performance if you would use bit-shifting, but the code doesn't.

FoamyGuy commented 1 week ago

@bablokb I think it's just waiting for review / testing. Do you have access to one of these devices to test with?

I'm not familiar at all with this chip, and don't have one to test with. Are you familiar with the device? If so can you confirm the change is correct?

bablokb commented 1 week ago

Yes I have these devices (both SCD40 and SCD41). And the change is correct.

Nevertheless, I propose a change. @kolcz: could you replace the 2**16-1 with a constant? It should have been a constant in the first place, but when you are already changing it, then I would suggest to make it a constant.

kolcz commented 1 week ago

Sorry for the long response, but I have adventures with pre-commit checks. I have changed 2^16 -1 to exact value and added line comments with equations for clarity. I have made changes also in temperature_offset getter and setter. @bablokb please test the code now. @FoamyGuy I wait for your review.

bablokb commented 6 days ago

I successfully tested the new code.

@FoamyGuy I think you can go ahead and merge the PR.