Fix some DisplayIO crashes, add ESP32-S2 debugging capability #3392
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tannewt
requested changes
Sep 11, 2020
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes null reference exceptions in DisplayIO related to the use of I2CDevice, which were causing crashes if the user entered the wrong I2C address for an I2C OLED screen, or did not include a reset pin in the construct function. These issues were identified by @ladyada in #3334, but were apparently occurring across all ports, not just the ESP32-S2.
This PR also adds an openocd configuration file and linker flag required to use GDB with the ESP32-S2, which assisted in tracking this issue. Correct behavior has been tested and confirmed using the I2C OLED Featherwing on the ESP32-S2 Saola Wrover and STM32F405 Feather Express. Test script here.
I snooped around DisplayIO a little looking for similar possible null reference exceptions. The only definite one I found was for the optional FourWire reset pin, but I'm wary of the Fourwire reset and other DisplayIO resets that assume a valid display object - there's no specific error that "skips" setting required variables in the construct function, the way there was for I2CDisplay, but maybe there are other ways to create an incomplete displayio object? It might be worth adding in a bunch of pointer validity checks, even if they're redundant most of the time.
Resolves #3334