Skip to content

Conversation

@kattni
Copy link
Contributor

@kattni kattni commented Nov 13, 2019

The way I initially created this check was not sufficient. The current way it works, it fails with "Sensor not found" if you hold an object too close to the sensor. It turns out that UART will return None under multiple situations: no sensor found, wiring incorrect, object too close to sensor. I worked with @dhalbert to debug the issue and determined that simply checking for if not data was not robust enough. With Dan's help, we created a much more robust check that allows for a single failed read, includes a delay to handle the fact that the sensor can't handle quick successive reads, and instead of throwing an error, returns None. I also updated the len(data) != 2 check to return None. This means in the event of that failure, the user would not need to include try/except code to have the sensor code continue running.

I was able to get the sensor to return None for _uart.read under all three listed situations: incorrect wiring, sensor not found and an object too close to the sensor. This is what triggered me requesting assistance to update the check because placing my hand over the sensor was triggering the current RuntimeError: Sensor not found. Check wiring!.

I was not however able to get the sensor to fail the len(data) != 2 check by either moving it around very quickly, or pointing it at the sky with no object in front of it. Moving it around quickly returns various distances. Pointing it at the sky returned 1122.9 for distance, and did not fail. I can not verify that the explanation in the docstring for this check is accurate, but I did not remove it as it was included in the original. I simply updated it to explain that it returns None.

Fixed what I believe to be typo in the docstring - I believe it was meant to say the sensor can not detect objects over 460 cm away (the "datasheet" states max 450, so this approximately matches what I assume the docstring was meant to say).

@kattni kattni requested review from dhalbert and ladyada November 13, 2019 21:42
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

👍

@dhalbert dhalbert merged commit c1e9eb7 into adafruit:master Nov 23, 2019
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 23, 2019
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.

3 participants