Respect StatusLED configuration at ShutdownLED when shutdown sequence is cancelled and the led share the same GPIO #1997

topas-rec · 2023-03-18T08:43:50Z

fixes 🐛 | StatusLed stays off when ShutdownButton LED is activated and shutdown request is aborted by releasing the shutdown button before the hold time expired #1983

I would like to keep the led on when the shutdown sequence (holding the shutdown button for the "hold time") is cancelled and a StatusLED is configured.
It should be off when the sequence is cancelled and no StatusLED is configured.

components/gpio_control/GPIODevices/shutdown_button.py

components/gpio_control/gpio_control.py

topas-rec · 2023-03-18T09:25:26Z

Yeah, like this so far.
How can this be improved by using object oriented approach?

s-martin · 2023-03-26T08:56:52Z

Thanks for this PR! 🎉

could you check the Python unit tests, which are currently failing and try to fix them in this PR?
https://github.com/MiczFlor/RPi-Jukebox-RFID/actions/runs/4454707894/jobs/7824072075?pr=1997

topas-rec · 2023-03-28T18:00:51Z

Thanks for this PR! tada

could you check the Python unit tests, which are currently failing and try to fix them in this PR? https://github.com/MiczFlor/RPi-Jukebox-RFID/actions/runs/4454707894/jobs/7824072075?pr=1997

Thanks for looking into it.
I was thinking out loud in this PR so far and this is not complete and I don't expect the checks to pass. I also did not test anything in here.
I converted this to draft now and I hope that the checks are not running any longer.

I created this PR, because I wanted to start with fixing it, though I have no clear way on how to implement it. Doing something sometimes helps rather than thinking days about it.
I appreciate help on how to integrate this fix properly.
If you can, try to understand my thoughts and notes above and look at the current progress. It's not much and the fix is not hard to do, but there are some design decisions to make. I am happy to hear some hints.

I plan to continue this when I find some free time.

components/gpio_control/gpio_control.py

topas-rec · 2023-05-20T06:48:14Z

Tested in unit tests so far and I am happy with it.
Final test with this code on the real hardware has to be done with the following configurations in gpio_settings.ini:

StatusLED and ShutdownButton not present (Just to see if there are errors)
- Result: StatusLED** off, ShutdownLED not blinking when shutdown button pressed
StatusLED present and ShutdownButton absent
- Result: StatusLED on, ShutdownLED not blinking when shutdown button pressed
StatusLED absent and ShutdownButton present
- Result: StatusLED off, ShutdownLED blinking when shutdown button pressed
StatusLED and ShutdownButton present, but sharing a different LED (Cannot really test it - I only have one LED)
- Result: StatusLED unknown (different pin - not connected), ShutdownLED blinking when shutdown button pressed
StatusLED and ShutdownButton present and sharing the same LED (This test tests issue 🐛 | StatusLed stays off when ShutdownButton LED is activated and shutdown request is aborted by releasing the shutdown button before the hold time expired #1983)
- Result:
- StatusLED on, ShutdownLED blinking when shutdown button pressed and stays on when shutdown aborted (fixes 🐛 | StatusLed stays off when ShutdownButton LED is activated and shutdown request is aborted by releasing the shutdown button before the hold time expired #1983 )
- Shutdown works, too
- On reboot, the power LED in on as it should

topas-rec · 2023-05-20T09:41:17Z

Should the test adoptions go into a separate commit? @s-martin ?

topas-rec · 2023-05-20T10:53:11Z

I think the tests never finish because of a while loop never succeeding in on the test systems.
See https://github.com/topas-rec/RPi-Jukebox-RFID/blob/71d05d79c357c5e7663628599bc9b69a04d878b3/components/gpio_control/GPIODevices/led.py#L39

s-martin · 2023-05-20T13:55:23Z

I think the tests never finish because of a while loop never succeeding in on the test systems.
See https://github.com/topas-rec/RPi-Jukebox-RFID/blob/71d05d79c357c5e7663628599bc9b69a04d878b3/components/gpio_control/GPIODevices/led.py#L39

Yeah, that could be the issue.

A method to overcome such scenarios could be mocking the methods during the tests.

please have a look here for starters: https://docs.python.org/3/library/unittest.mock-examples.html

topas-rec · 2023-05-25T12:42:36Z

I already mocked it by adding a mock inside that file led.py. Then the test works.
I added it in there because it has an include for os.system which shadows (I think) the mock for the system function from the file that holds the tests .

Mocking inside the file led.py would then be present in production.

topas-rec · 2023-05-27T05:58:35Z

components/gpio_control/GPIODevices/led.py

+from mock import MagicMock
+
+system = MagicMock(return_value=0)


Now the test works, but the code doesn't work in production anymore I think.

I can't get the test to work when I mock in https://github.com/topas-rec/RPi-Jukebox-RFID/blob/305aa26bec34ca18b8a248d5aff7e502e9a0e7c4/components/gpio_control/test/test_gpio_control.py

* Test configuration file had still `functionCallDown` and `functionCallUp` as function calls instead of newer `functionCall1` and `functionCall2` * Made path of config file relative to the location from where the imports work (and the test is executed which is `./components/gpio_control/`

* Test didn't finished without this setting since os.system is used to check if a phoniebox service is running on the machine which may not run on machines this unit tests run on

* Code not working * Base for discussion

… this value

* untested * If a ShutdownButton and a StatusLed are both configured and use the same led pin, the status that the led should have in case a shutdown request (blinking shutdown led) is cancelled is set at the ShutdownButton

topas-rec · 2023-05-27T19:22:48Z

Made mocking work in #2034.
Merge this after #2034.

topas-rec changed the title ~~Aborting shutdown sequence now checks for StatusLED configuration~~ Respect StatusLED configuration at ShutdownLED when shutdown sequence is cancelled and the led share the same GPIO Mar 18, 2023

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch from 6ef60c0 to b63b715 Compare March 18, 2023 08:58

topas-rec commented Mar 18, 2023

View reviewed changes

components/gpio_control/GPIODevices/shutdown_button.py Outdated Show resolved Hide resolved

topas-rec commented Mar 18, 2023

View reviewed changes

components/gpio_control/gpio_control.py Show resolved Hide resolved

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch from af11c7d to 6af1a76 Compare March 18, 2023 09:28

s-martin added the enhancement label Mar 26, 2023

topas-rec marked this pull request as draft March 28, 2023 17:48

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch from 6af1a76 to 00a51fd Compare May 10, 2023 18:24

s-martin reviewed May 10, 2023

View reviewed changes

components/gpio_control/gpio_control.py Outdated Show resolved Hide resolved

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch 2 times, most recently from 8713fe4 to 7dd5364 Compare May 10, 2023 19:30

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch 2 times, most recently from ba4558e to 71d05d7 Compare May 20, 2023 06:33

topas-rec marked this pull request as ready for review May 20, 2023 09:37

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch 2 times, most recently from 50e6290 to 71d05d7 Compare May 27, 2023 05:53

topas-rec commented May 27, 2023

View reviewed changes

topas-rec marked this pull request as draft May 27, 2023 06:01

topas-rec added 4 commits May 27, 2023 20:59

Mocked function os.system in test_gpio_control

ecf4670

* Test didn't finished without this setting since os.system is used to check if a phoniebox service is running on the machine which may not run on machines this unit tests run on

Aborting shutdown sequence now checks for StatusLED configuration

b4bdfe8

* Code not working * Base for discussion

Add dependency check for devices

51364e9

topas-rec added 2 commits May 27, 2023 21:00

Set the led state for shutdown cancellation in ShutdownButton and use…

fa2fecb

… this value

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch from 305aa26 to 54580f5 Compare May 27, 2023 19:04

Added devices dependency check to test test_gpio_control

e51652e

topas-rec force-pushed the fix-statusLed-stays-off-when-ShutdownButton-LED-is-activated-and-shutdown-request-is-aborted-by-releasing-the-shutdown-button-before-the-hold-time-expired branch from 54580f5 to e51652e Compare May 27, 2023 19:24

s-martin added the legacy_v2 Issues, discussions and PRs related to Version 2.x label Dec 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respect StatusLED configuration at ShutdownLED when shutdown sequence is cancelled and the led share the same GPIO #1997

Respect StatusLED configuration at ShutdownLED when shutdown sequence is cancelled and the led share the same GPIO #1997

topas-rec commented Mar 18, 2023

topas-rec commented Mar 18, 2023

s-martin commented Mar 26, 2023

topas-rec commented Mar 28, 2023

topas-rec commented May 20, 2023 •

edited

topas-rec commented May 20, 2023

topas-rec commented May 20, 2023 •

edited

s-martin commented May 20, 2023

topas-rec commented May 25, 2023

topas-rec May 27, 2023

topas-rec May 27, 2023

topas-rec commented May 27, 2023

		from mock import MagicMock

		system = MagicMock(return_value=0)

Respect StatusLED configuration at ShutdownLED when shutdown sequence is cancelled and the led share the same GPIO #1997

Are you sure you want to change the base?

Respect StatusLED configuration at ShutdownLED when shutdown sequence is cancelled and the led share the same GPIO #1997

Conversation

topas-rec commented Mar 18, 2023

topas-rec commented Mar 18, 2023

s-martin commented Mar 26, 2023

topas-rec commented Mar 28, 2023

topas-rec commented May 20, 2023 • edited

topas-rec commented May 20, 2023

topas-rec commented May 20, 2023 • edited

s-martin commented May 20, 2023

topas-rec commented May 25, 2023

topas-rec May 27, 2023

Choose a reason for hiding this comment

topas-rec May 27, 2023

Choose a reason for hiding this comment

topas-rec commented May 27, 2023

topas-rec commented May 20, 2023 •

edited

topas-rec commented May 20, 2023 •

edited