Skip to content

Fix bug in reset that wasn't setting status properly #26

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 1 commit into from
Jan 21, 2021

Conversation

chrisbailey4
Copy link
Contributor

Removed setting the _status_last to the return value as the return result isn't just the status field.
The _transceive function is actually setting the _status_last as part of the function so there is no need to do anything additional in reset

This addresses Issue: #25

Removed setting the `_status_last` to the return value as the return result isn't just the status field.
The `_transceive` function is actually setting the `_status_last` as part of the function so there is no need to do anything additional in reset
@tannewt tannewt requested a review from caternuson January 20, 2021 01:23
@caternuson
Copy link
Contributor

Looks good and agree it seems like an unnecessary, and potentially incorrect, setting of status.

Did you test this code with the original issue to make sure it fixed things there? Seems like this bug could be demonstrated by simply calling display_status after a call to reset?

@chrisbailey4
Copy link
Contributor Author

Hi @caternuson I did test the code with the original issue and confirmed it addressed my issue.

The Bug is slightly more complex then just calling display_status after reset... In my specific case, I was attempting to enable the burst mode on the chip and I managed to get the chip into a state where it would properly respond to the self._transceive(bytes([_CMD_RT])) call in the reset function. However I would get an exception, IO error, during the self.magnetic call. In a normal reset call the self._status_last is properly set in the magnetic call, so the fact that it wasn't set in the call above would hardly ever been experienced.

This truly is a minor case that I hope not many people experience, but for those that do I am happy to provide this fix to save them 30 minutes of debugging :-)

@caternuson
Copy link
Contributor

Gotcha. Yep, that makes sense. That sneaky call to magnetic was normally masking the issue. Subtle little bug. Thanks for discovering and fixing this.

@caternuson caternuson merged commit 0ff3dcb into adafruit:master Jan 21, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 23, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 4.2.1 from 4.2.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#100 from adafruit/REUSE
  > Added pre-commit-config file
  > Added pre-commit and SPDX copyright
  > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#96 from adafruit/tap-matching

Updating https://github.com/adafruit/Adafruit_CircuitPython_EPD to 2.7.1 from 2.7.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_EPD#44 from adafruit/REUSE
  > Added pre-commit-config file
  > Added pre-commit and SPDX copyright

Updating https://github.com/adafruit/Adafruit_CircuitPython_IL0373 to 1.3.3 from 1.3.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_IL0373#19 from adafruit/REUSE
  > Merge pull request adafruit/Adafruit_CircuitPython_IL0373#18 from peterhinch/patch-1
  > Added pre-commit-config file
  > Added pre-commit and SPDX copyright

Updating https://github.com/adafruit/Adafruit_CircuitPython_LSM9DS1 to 2.1.5 from 2.1.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_LSM9DS1#28 from adafruit/REUSE
  > Added pre-commit-config file
  > Added pre-commit and SPDX copyright

Updating https://github.com/adafruit/Adafruit_CircuitPython_MLX90393 to 2.0.3 from 2.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_MLX90393#26 from chrisbailey4/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 4.1.0 from 4.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#55 from flavio-fernandes/socket_read_fixes

Updating https://github.com/adafruit/Adafruit_CircuitPython_RSA to 1.2.1 from 1.2.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_RSA#17 from adafruit/REUSE
  > Added pre-commit-config file
  > Added pre-commit and SPDX copyright

Updating https://github.com/adafruit/Adafruit_CircuitPython_TinyLoRa to 2.2.0 from 2.1.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_TinyLoRa#36 from imliubo/master
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.

2 participants