Skip to content
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

epdconfig.py closes device so sleep doesn't work #315

Open
robweber opened this issue Nov 28, 2023 · 11 comments
Open

epdconfig.py closes device so sleep doesn't work #315

robweber opened this issue Nov 28, 2023 · 11 comments

Comments

@robweber
Copy link

Looks like PR #314 changed the way the module_init and module_exit classes behave which results in an error when utilizing the display sleep functionality.

When running as part of a loop the EPD will display an image on the first run. When trying to display a new image after going to sleep an error is thrown GPIODeviceClosed. Comparing the old and current versions of the file it looks like the previous version would setup each pin as part of the module_init method and then close them with module_exit. In the current version the pins are setup in the init() method instead. When waking up from sleep the existing object calls module_init which then tries to perform an action on an already closed device.

The result is that makes reusing the EPD class object impossible and makes the sleep function useless. Refactoring epdconfig.py to utilize the old behavior of setting up the pin in the module_init method would probably fix this. Log below of this happening.

File "/home/administrator/vsmp-plus/.venv/lib/python3.11/site-packages/omni_epd/displays/waveshare_display.py", line 129, in prepare
    self._device.init()
  File "/home/administrator/vsmp-plus/.venv/lib/python3.11/site-packages/waveshare_epd/epd7in5_V2.py", line 87, in init
    if (epdconfig.module_init() != 0):
        ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/administrator/vsmp-plus/.venv/lib/python3.11/site-packages/waveshare_epd/epdconfig.py", line 102, in module_init
    self.GPIO_PWR_PIN.on()
  File "/usr/lib/python3/dist-packages/gpiozero/output_devices.py", line 210, in on
    self._write(True)
  File "/usr/lib/python3/dist-packages/gpiozero/output_devices.py", line 89, in _write
    self._check_open()
  File "/usr/lib/python3/dist-packages/gpiozero/devices.py", line 583, in _check_open
    raise GPIODeviceClosed(str(e))
gpiozero.exc.GPIODeviceClosed: LED is closed or uninitialized
@PainOchoco
Copy link

Hello, I got the same issue. Have you tried to revert the changes from PR #314?

@robweber
Copy link
Author

robweber commented Dec 6, 2023

Yes that's pretty much what I did to get it working again. This fork of the Waveshare repo has the Rpi Bookwork fixes but not the changes from #314 so I've just been using that.

@txoof
Copy link
Contributor

txoof commented Dec 17, 2023

@shhds
Is there any chance you can either revert the changes made in PR #314, or address this? The fork mentioned above works for my cases, but I'd rather not depend on an out-of-date fork if I can avoid it.

Thanks for your help!

@pgeschwill
Copy link

I agree, this is not ideal.

The issue stems from the move to gpiozero from RPi.GPIO.
According to gpiozero's documentation, the library automatically does a cleanup of the used Pins after the script has executed. This removes the need to explicitly run a GPIO.cleanup as in RPi.GPIO.
However, epdconfig.py calls the close method on a couple of pins which makes them unusable after the first use.

I managed to work around this by just commenting out the calls to the close method. I believe this is fine as long as your script terminates properly because of the aforementioned automatic cleanup.
However, this creates the need for adding proper error handling to your own script which does the clean up explicitly in case the script terminates abnormally.

@txoof
Copy link
Contributor

txoof commented Dec 18, 2023

@pgeschwill & @robweber
I've got a proof of concept that resolves this issue. Would you mind taking a look at it before I make a PR?

The optional_cleanup branch adds an the argument cleanup to the module_exit() method.

epdconfig.py

I've implemented it for all the modules and the tests. I only have a 5in83 and 5in65f to test with at the moment, but it looks OK for those under Bookworm.

On an unexpected exit (e.g caught keyboard interrupt), you can call module_exit(cleanup=True) The rest of the time, you can just depend on the gpiozero automatic cleanup.

@txoof txoof mentioned this issue Dec 20, 2023
@txoof
Copy link
Contributor

txoof commented Dec 20, 2023

In the past, sending an email has helped raise the priority of issues with the E-Paper team. I think they're really busy with many projects.

I sent the following mail on 20 December:

Waveshare E-Paper Team,

The changes in PR#314 corrected some problems with Raspberry Pi 5 and Bookworm, but introduced a change to the API for the sleep method.

The sleep method now closes the board and requires that the program is restarted before additional writes. This causes a lot of problems for anyone downstream. This is documented in Issue #315.

Would you please consider PR#318 as a fix for this issue?

Thank you for your help. The community appreciates all that you do!

@robweber
Copy link
Author

@txoof - I didn't field test the PR you proposed but reviewing the code it seems sound. It's too bad implementers will have to add the error handling themselves to fix this but it's easily done. This fixes the main issue fairly easily without having to refactor a lot of other code or add additional logic to epdconfig.py.

@pgeschwill
Copy link

@txoof : Amazing work, thanks for your effort! I have added a comment to the PR.

@txoof
Copy link
Contributor

txoof commented Dec 20, 2023

@pgeschwill
Good catch. I moved the if statement in epdconfig.py

I've tested it out on the screens that I have with some test code, but I'd love another pair of eyes.

@shhds
Copy link
Contributor

shhds commented Dec 21, 2023

Thank you very much for your feedback, we will probably fix this issue in the next version; An update is expected within a week

@txoof
Copy link
Contributor

txoof commented Jan 6, 2024

@robweber The proposed changes in PR #318 were merged in outside of the PR. I'll get to testing it in the near future.

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

No branches or pull requests

5 participants