SPI().deinit() fix

[bill88t]

When the board.SPI() bus is deinitialised, it will fail on the next display write with a valuerror and deinit the display.
The error already exists, so no binary size increase.

Closes #8278

Tested on my display-swapped feather s3 tft.

[jepler]

Thanks for working on fixing this issue!

Unfortunately, I don't think the fix is correct / complete.

In testing with this PR, I was able to get two different kinds of misbehavior, apparently depending on whether I had a code.py or not.

In one case, after I did board.SPI().deinit() and exiting the REPL, my code.py restarted and the ValueError was raised from an arbitrary spot:

Adafruit CircuitPython 8.2.0-77-g8534daeb87 on 2023-08-15; Adafruit Camera with ESP32S3
>>> board.DISPLAY.auto_refresh = False
>>> board.SPI().deinit()
>>> 
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.
code.py output:
Traceback (most recent call last):
  File "code.py", line 6, in <module>
  File "adafruit_pycamera.py", line 7, in <module>
  File "adafruit_debouncer.py", line 32, in <module>
  File "adafruit_ticks.py", line 56, in <module>
ValueError: Object has been deinitialized and can no longer be used. Create a new object.

In the other case, I got a Safe Mode reset:

Adafruit CircuitPython 8.2.0-77-g8534daeb87 on 2023-08-15; Adafruit Camera with ESP32S3
>>> import board
>>> board.DISPLAY.auto_refresh = False
>>> board.SPI().deinit()
>>> 
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

Code done running.

[09:30:53.402] Disconnected
[09:30:54.403] Warning: Could not open tty device (No such file or directory)
[09:30:54.403] Waiting for tty device..
[09:30:55.404] Connected

Auto-reload is off.
Running in safe mode! Not running saved code.

You are in safe mode because:
CircuitPython core code crashed hard. Whoops!
Heap allocation when VM not running.
Please file an issue with your program at https://github.com/adafruit/circuitpython/issues.
Press reset to exit safe mode.

Press any key to enter the REPL. Use CTRL-D to reload.

I thought that maybe instead of mp_raise_ValueError just return false would fix things, but that's not a complete fix either. With that, I got

Adafruit CircuitPython 8.2.0-77-g8534daeb87-dirty on 2023-08-15; Adafruit Camera with ESP32S3
>>> import board
>>> board.SPI().deinit()
>>> 
[09:37:58.844] Disconnected
[09:37:59.844] Warning: Could not open tty device (No such file or directory)
[09:37:59.844] Waiting for tty device..
[09:38:00.846] Connected

Auto-reload is off.
Running in safe mode! Not running saved code.

You are in safe mode because:
CircuitPython core code crashed hard. Whoops!
Fault detected by hardware.
Please file an issue with your program at https://github.com/adafruit/circuitpython/issues.
Press reset to exit safe mode.

[bill88t]

In one case, after I did board.SPI().deinit() and exiting the REPL, my code.py restarted and the ValueError was raised from an arbitrary spot

The way I implemented it, it should only be raised on the next screen write. Aka, the next full line.
It may be that the data is still written while the display is not refreshing, causing the behaviour you saw.
I didn't read the whole displayio source.

In the other case, I got a Safe Mode reset

Here, it seems like the error was raised outside of the VM, as the soft reboot text was attempted to be printed.
The most probable cause for the allocation, are the pin resets that occur for the displayio object's deinit.
If I disable them the displayio object will leave these pins allocated and the ValueError will be raised repeatedly (so we want the deinit to happen).
Perhaps I need a way to check if the vm is running.

Thank you for your throughout testing! I didn't think of these cases.

[bill88t]

In the other case, I got a Safe Mode reset

This only occurs at the rare edge case you have no code.py, at all.

[bill88t]

Now if the spi bus is deinit'ed the display is not reset.
The ValueError was removed. Having a surprise ValueError on a print() is not a good idea.
I suggest we put there a new serial message instead (I am not going to decide the content of said message).

Now doing:

import board
board.DISPLAY.auto_refresh = False
board.SPI().deinit()
^D

Results in a hang. I have no clue where. No jtag on S3.

I should note that properly solving this edge case will take up a lot of code. I am not sure it's worth it.

[bill88t]

Okay, now it works as expected.
Currently does not show an error message or raise any exceptions.
I do not want to decide on an error message. But I suggest it's a serial_write_compressed instead of an exception.

This:

try:
    print()
except ValueError:
    pass

feels dystopian and I don't want to be responsible for it.

[Neradoc]

I think it would make more sense to simply raise an exception when trying to deinit a bus used by displayio.
No weird side effect at a future point in time.
If you want to free the display's bus, use displayio.release_displays()

[bill88t]

This PR exists solely for the purpose of fixing the hardfault.
As a matter of fact, deiniting a board.SPI bus breaks it completely, even across reloads.

So.. we are not fixing something broken into something working.
We are fixing something very broken, into something less broken.

A complete "fix", would be outright denying the deinit from going through.
A board bus property should perhaps not be "deinit-able" at all.
Or any SPI bus would require no displays to be attached to it to be "deinit-able".
But currently there is no way to prevent a deinit.

Implementing a "block-deinit" property that displayio or main (which does the bus allocs) could use, would require new busio functions, similar to how I imagined it originally in #8278.

But that goes against the requested fix, so I can't go do that unless I receive some form of consent.

Also, having an asyncronous-exception-thingy blasted at the user when they least expect it, doesn't seem helpful.
No user will ever anticipate it unless they read the source.

And on top of that an exception cannot be raised outside of the VM, reintroducing the previous hardfaults under the edge-case Jepler found. There is no clear solution to this, as displayio code, cannot know the vm state with the currently existing functions.

[bill88t]

Closing this, as there isn't a clear solution I can implement for it and there isn't any interest for this either way.

Maybe in the future I will try making board. stuff not be deinitializable, but that's a seperate thing.