-
Notifications
You must be signed in to change notification settings - Fork 52
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
Panic Codes don't match the micro:bit V1 #191
Comments
@finneyj Do you have some comment on this? I'm inclined to update these to match the web documentation on the help pages and the original DAL messages wherever possible/appropriate... was there a reason that these diverged, or is it just error-code-rot? |
When we generalized microbit-dal into codal-core, we dropped a lot of device specific content for obvious reasons, with the concept that additional constants could be layered back on the the device level repos (codal-microbit-v2 etc). It looks a bit like we did that, then the USB error sneaked into the gap that was left in codal-core... Suggest we generalize 50 to be "hardware error", then map in the V1 codes to that? (especially seeing as v2 boards have combined acc/mag sensors anyway)? Perhaps we could also 51 as I2C failure, 52 for USB? What thinks @microbit-carlos @JohnVidler |
I really must remember to sign out of my education account :) |
Check that panic codes have no hard coded constants in their classes for speicific devices, otherwise check the panic codes align. |
See the above merged PRs for how this has been addressed, but essentially I've now repurposed the 50-59 panic range to be board/implementation specific for peripheral device failures. In the case of the Micro:bit v2 we now have this enum:
in Closing as complete, but happy to address issues that may arise from this in new issues filed with details of the boards and errors it causes (although I personally expect that this will cause negligable problem cases) |
Reopening as we need the panic codes to be 50/51, not 51/52. This also needs to be fixed: #216 (comment) |
Marking as complete, as I've moved these back to 50/51. The comment @microbit-carlos mentions will be tracked in its own issue. |
V1 defined in the DAL repo: https://github.com/lancaster-university/microbit-dal/blob/602153e9199c28c08fd2561ce7b43bf64e9e7394/inc/core/ErrorNo.h#L72-L89
V2 might be using the values from the codal-core repo:
https://github.com/lancaster-university/codal-core/blob/befa2c51b3595ca0d44e1bc5daad0531974b77e7/inc/core/ErrorNo.h#L80-L105
The text was updated successfully, but these errors were encountered: