adafruit / Adafruit_CircuitPython_MLX90640

A pure Python MLX90640 driver
MIT License
65 stars 20 forks source link

Handle broken pixels #18

Closed fgervais closed 4 years ago

fgervais commented 4 years ago

I got a mlx90640 with a defective pixel and even though the datasheet says that it's acceptable up to 4, the library was not able to recover and was crashing when trying to calculate the forth root of a negative number:

Traceback (most recent call last):
  File "mlx90640_camtest.py", line 124, in <module>
    raise e
  File "mlx90640_camtest.py", line 121, in <module>
    mlx.getFrame(frame)
  File "/home/fgervais/Adafruit_CircuitPython_MLX90640/adafruit_mlx90640.py", line 159, in getFrame
    self._CalculateTo(mlx90640Frame, emissivity, tr, framebuf)
  File "/home/fgervais/Adafruit_CircuitPython_MLX90640/adafruit_mlx90640.py", line 341, in _CalculateTo
    Sx = math.sqrt(math.sqrt(Sx)) * self.ksTo[1]
ValueError: math domain error

I added the missing code to handle broken adjacent pixels that had been left out from the library port (noted as TODO) and added the code to detect and "fix" broken pixels when doing the conversion to Celsius.

I must say, I made the dumbest fix but hey, at least it's a valid option recommended by the datasheet :). If I end up needing the more advanced interpolation fixes I'll gladly PR them later on.

fgervais commented 4 years ago

Any suggestions about the linter errors?

ladyada commented 4 years ago

see https://learn.adafruit.com/improve-your-code-with-pylint for a guide!

fgervais commented 4 years ago

I see. Well I get the errors but let say "Method could be a function". I agree but I still feel like it makes more sense to have it in the class since it's functionally related and also there is no other functions in the module currently so I feel it would look weird to add two lonely little functions.

Then "Too many branches", I think this part fits well with the code already in place and what mlx90640-library has which everything seems to be translated from.

Lastly "Simplify chained comparison between the operands", well I kept it close to the mlx90640-library. I feel the code is pretty clear too.

From my point of view I would ignore all those but I can change any part of the code so it suits your tastes better.

fgervais commented 4 years ago

Should I put the utility method _UniqueListPairs() outside of the class?

jepler commented 4 years ago

Thank you @fgervais !

fgervais commented 4 years ago

I'm glad I could help :)