search

basnijholt / miflora

☘️🌡🌼🥀🏡 Mi Flora Plant sensor Python package
MIT License
363 stars 98 forks source link

Changed formula for temperature to include negatives #59

Closed gulasnani closed 6 years ago

ThomDietrich commented 6 years ago

Rationale?

Edit: this was the late-night version of what @ChristianKuehnel posted below ;) Additions to open source projects are ALWAYS welcomed but it is often a good idea to describe the contribution.

ChristianKuehnel commented 6 years ago

@gulasnani that you for the PR. 😁

Please also add some unit test for this where you decode some byte array, you recieved from the sensor.

I did not know the unpack function, thank you for the hint. Looking at the documentation, shouldn't it be "<H" (capital H) for signed numbers?

Did you check if the right encoding for signed numbers is used?

Should we use a longer unpack string to decode the entire byte array in one step?

gulasnani commented 6 years ago

@ThomDietrich Really sorry for that, since I am quite a newbie. The rationale behind using unpack is that we let python decode the string as a short signed int through the format string provided as the first argument to the function. This saves us the headache of converting the bits ourselves. @ChristianKuehnel Sure, I will look into the testing. I checked the documentation again, and it says 'h' is for signed short int. And yeah, even I have had the opinion that it would be the easiest to use unpack for the entire string code :D

ChristianKuehnel commented 6 years ago

Sorry, you were right "<h" was correct.

ChristianKuehnel commented 6 years ago

This is for issue https://github.com/open-homeautomation/miflora/issues/49

ChristianKuehnel commented 6 years ago

Thx for your input. I used your code and also added tests: https://github.com/open-homeautomation/miflora/pull/65