Skip to content

Handle broken pixels #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 9, 2020
Merged

Handle broken pixels #18

merged 4 commits into from
Sep 9, 2020

Conversation

fgervais
Copy link

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
Copy link
Author

Any suggestions about the linter errors?

@ladyada
Copy link
Member

ladyada commented May 13, 2020

@fgervais
Copy link
Author

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
Copy link
Author

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

@kattni kattni requested a review from a team August 19, 2020 19:21
Copy link
Contributor

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discord user milinile reports that this PR addresses the problem for them.

@jepler jepler merged commit fb82a79 into adafruit:master Sep 9, 2020
@jepler
Copy link
Contributor

jepler commented Sep 9, 2020

Thank you @fgervais !

@fgervais
Copy link
Author

fgervais commented Sep 9, 2020

I'm glad I could help :)

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Sep 11, 2020
Updating https://github.com/adafruit/Adafruit_CircuitPython_MLX90640 to 1.2.0 from 1.1.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_MLX90640#21 from adafruit/fix-pylint
  > Merge pull request adafruit/Adafruit_CircuitPython_MLX90640#18 from fgervais/broken-pixels

Updating https://github.com/adafruit/Adafruit_CircuitPython_PyPortal to 3.4.0 from 3.3.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#83 from cjsieh/scard_vs_sdcardio

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE to 7.3.0 from 7.2.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#103 from dhalbert/hci

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_BerryMed_Pulse_Oximeter to 2.0.2 from 2.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_BerryMed_Pulse_Oximeter#4 from dhalbert/up-to-date-data

Updating https://github.com/adafruit/Adafruit_CircuitPython_framebuf to 1.4.1 from 1.3.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#35 from makermelissa/master
  > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#34 from makermelissa/master
  > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#33 from jsharper/fix/jsharper/image-rotate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants