pybricks
Power off improvements
This is an attempt to do a better job shutting off all I/O port devices when powering off the hub.
The last commit fixes the issue identified in https://github.com/orgs/pybricks/discussions/1795#discussioncomment-10507805.
The middle commit fixes sensors like the BOOST Color/Distance sensor staying on while the button is held down at power off.
The first commit is setting up for the second commit.
I tested briefly on City Hub and SPIKE Prime, but this could use some more @BertLindeman level QA testing :tm: :smile:
- We want to make sure we don't break the test case from https://github.com/pybricks/support/issues/385
- It seems like some city hubs are worse than other about coming back on after releasing the power button, so we want to make sure that didn't regress
Download the artifacts for this pull request:
- pr_number.zip
- mpy-cross.zip
- pybricks-micropython-build-3542-git068d2cae.zip
- movehub-firmware-build-3542-git068d2cae.zip
- cityhub-firmware-build-3542-git068d2cae.zip
- nxt-firmware-build-3542-git068d2cae.zip
- essentialhub-firmware-build-3542-git068d2cae.zip
- primehub-firmware-build-3542-git068d2cae.zip
- technichub-firmware-build-3542-git068d2cae.zip
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
coverage: 56.033% (-0.009%) from 56.042% when pulling 91c59400b374ddf8e7f2317044b599660ed638bd on dlech:power-off-improvements into bd7c3450a272fad5b54794555e6241c44512f2b6 on pybricks:master.
Tested movehub at build 3514. Color/distance. Press button and keep pressed. Hub goes off and sensor re-lights again. Cityhub at 3514 does the same. Could not trigger this on spike prime / Robot Inventor / Technic hub.
[EDIT] added Technic hub. Going to test with build 3517
City hub no regression seen on issue 385 no re-power-on on the Color-Distance sensor.
Movehub same positive result.
Technichub same positive result.
A-Oh Regression: Spike at build 3517 with USB connected after power-off does light the Color sensor and the Color-distance sensor again. And also with the program for issue 385 the error is back:
from pybricks.hubs import ThisHub
from pybricks.tools import wait
hub = ThisHub()
print(hub.system.reset_reason())
wait(100)
hub.system.shutdown()
The hub mimics "I am powered off" but the Color sensor and the Color-distance sensor flip-flop off and on at ~ 1 Hz?
David as I do not know if you see updated comments, so I add this comment to let you know I finished this tests.
Bert
{EDIT] movie of Spike prime hub power-off while USB connected to loader:
https://github.com/user-attachments/assets/72c5b8c8-7925-4481-9ade-d302ad742611
Hi @BertLindeman, thanks for all the testing!
Press button and keep pressed. Hub goes off and sensor re-lights again.
I'm not able to reproduce this. Here is what I am doing:
- Attach sensor to City hub or Move hub
- Press and release button to turn on hub
- Press and hold button to turn off hub
- Keep holding button for 10 sec to see if issue reproduces
- All lights stay off
- Release button
Spike at build 3517 with USB connected after power-off does light the Color sensor and the Color-distance sensor again.
I am able to reproduce this issue.
And also with the program for issue 385 the error is back:
Which error are you referring to here?
And also with the program for issue 385 the error is back:
Which error are you referring to here?
What I referred to was the Spike hub. As you reproduced.
Not the city hub.
Sorry for the confusion. Issue 365 is not back.
Spike at build 3517 with USB connected after power-off does light the Color sensor and the Color-distance sensor again.
This issue should be fixed now (build 3526).
Press button and keep pressed. Hub goes off and sensor re-lights again.
I'm not able to reproduce this. Here is what I am doing:
Ah, I think you were referring to a build from before this pull request. So all good now?
Test-mode ON All at build 3526 run the issue_385 program and verify no re-power-on
- Spike-prime USB connected - OK
- technichub - OK
- cityhub - OK
- movehub - OK
Long press power button if hub is ON and keep pressed and verify color sensor and color-distance sensor stay OFF (No spike 3x3 matrix available here)
- Spike-prime USB connected - OK
- technichub - OK
- cityhub - OK
- movehub - OK
OK for me, David.
David, Ran build 3527.
scenario: spike prime or Robot Inventor:
- run issue_385 program (see some post before this) with USB connected to load the battery
- run the program (F5), the hub does a shutdown, all OK
- set the hub on again and run the program by a button press. (Still USB connected!) Now the Bluetooth button stays blue. This occurs also on build 3514 (I just happened to have available)
Long press to power-off still does put off all lights.
No idea how come, Laurens did not implement the light refactor, did he?
Bert
No idea how come, Laurens did not implement the light refactor, did he?
There were some changes to the Bluetooth button merged already, so that is probably a side effect of those changes. So hopefully that will be fixed by one of the other open pull requests.
Reported on https://github.com/orgs/pybricks/discussions/1801, MoveHub still need work.
Thanks for the pull request!
Is it possible to make a short list of hub/sensor/usb poweroff permutations that this now fixes?
staying on while the button is held down at power off.
Is this the only issue or does something also stay on permanently when connected to USB?
Is this the only issue or does something also stay on permanently when connected to USB?
USB connected and power button pressed both keep the power on, so there is really no difference between those. If something stays on when USB is connected, then it will also stay on if the power button is pressed.
Is it possible to make a short list of hub/sensor/usb poweroff permutations that this now fixes?
I don't know all of the sensors that request power on P1 or P2, but these are affected. Before, power on P1 or P2 was left on, now it is turned off. This was only noticeable without a volt meter on the SPIKE color light matrix since it turned on all green because of this.
Before this, P4 was powered off when shutdown was requested (it doesn't matter if a device is connected or not). After this change, they don't power off until shutdown is complete. So now, the hub status light and the sensors turn off at the same time. Before, the sensors would turn off, then the hub status light would blink after that. The difference is only noticeable to the user if the sensor has a light.
Bug that still needs to be fixed. If Move hub has a sensor attached, all works as expected. But if Move hub doesn't have a sensor attached, it will turn itself back on after the button is released. This bug was most likely introduced by these changes.
Thanks for the overview David.
@BertLindeman -
In short: if the hub is not powered-off the power-led color is remembered or not reset.
Coincidentally, this was fixed in https://github.com/pybricks/pybricks-micropython/commit/6d65df1eb83de3fc19a50f7f569562946936f85c earlier today.
[EDIT] For clarity add a column to the table with the build 3537 result.
Power button press patterns.
-
Long power button press and release on power-off as soon as blinking stops
-
Long power button press and release after (half) a second? after blinking stops This is a tricky measurement. How long is not so long? Have seen observations where a pattern 2 test kept the hub off and later on again, with color-distance sensor on port-D
-
Long power button press and keep a few seconds (3 – 5)
Tests on movehub at build 3527 and build 3537
| description | attachment(s) | result 3527 | result 3537 |
|---|---|---|---|
| pattern 1 | -none- | hub switches ON | hub stays OFF – OK |
| pattern 1 | LED lights on C | hub switches ON | hub stays OFF – OK |
| pattern 1 | Color sensor on C | hub stays OFF – OK | hub stays OFF – OK |
| pattern 1 | Color/DIST sensor on C | hub switches ON | hub stays OFF – OK |
| pattern 2 | -none- | hub switches ON | hub stays OFF – OK |
| pattern 2 | LED lights on C | hub stays OFF – OK | hub stays OFF – OK |
| pattern 2 | Color sensor on C | hub stays OFF – OK | hub stays OFF – OK |
| pattern 2 | Color/DIST sensor on C | hub switches ON | hub stays OFF – OK |
| pattern 2 * | Color/DIST sensor on D | hub stays OFF sometimes !! | hub stays OFF – OK |
| pattern 3 | -none- | hub switches ON | hub switches ON |
| pattern 3 | LED lights on C | hub stays OFF – OK | hub stays OFF – OK |
| pattern 3 | Color sensor on C | hub stays OFF – OK | hub switches ON |
| pattern 3 | Color/DIST sensor on C | hub switches ON | hub switches ON |
Conclusion
If a color-sensor is connected to port-C all is OK, even if a color-distance sensor is on Port.D. If a LED-light is attached on Port-C with either nothing or a Color-Distance sensor on Port-D pattern 1 causes a hub switch ON again.
Repeat this on the current build, and added to the table.
Bert
OK, hopefully I got this working with all combinations now. Also rebased on current master, so other recent status light/bluetooth light changes should be included.
Amending my previous summary of changes, the most noticeable differences are:
- On Move hub and City Hub - before: any sensor with lights would stay on until the button was released on power off - after: the sensor lights turn off at the same time as the hub status light, even while the button is still pressed.
- All hubs plus color light matrix - before: the color light matrix would turn all green after a short time when the hub was "off" but still powered due to button pressed or USB connected - after: the color light matrix sensor stays off when the hub is "off", even when the hub is still powered due to button pressed or USB connected.
Will test using build 3542 for this PR
Yup, that should be the latest and greatest.
No 3x3 light matrix here, so not tested.
Maybe Manfred @msw-home can test? Related to discussion 1795 LED Matrix schaltet sich eigenständig wieder ein
Tested cityhub, movehub like the table above. All OK Similar with technichub and primehub. OK too.
Great David.
Bert
Thanks!
I'd like to get this merged in after reworking the port logic/processes.
We should be able to make shutdown a bit more a chronological process, taking in all the quirks here as dedicated (optionally enabled) steps in that process.
Thanks for looking into this. The upcoming ports overhaul should simplify this quite a bit.
Powering off the motor terminals (also for sensors) now happens gracefully since https://github.com/pybricks/pybricks-micropython/commit/a1cf040c9401741c37f8543946cc9a6c93ba0bfc.
It implements a call pbio_port_poweroff_is_ready that we include in the final shutdown loop just like is proposed in this PR. I still have to include the VCC powerdown quirks.
I am understanding correctly that the following quirk should no longer be used anywhere?
// Turn off power on pin 4 on all ports. This is set to input instead of
// low to avoid city/move hubs turning back on when button released.
// as soon as the user releases the power button
pbdrv_gpio_input(&pbdrv_ioport_pup_platform_data.port_vcc);
I see the following comments added throughout.
// Some hubs will turn themselves back on if VCC is off when power is
// turned off, so we have to turn VCC back on as a workaround.
// We want VCC off so that lights on sensors turn off while the power
// button is held down. But, this would cause the hub to turn back on
// immediately. So return true to indicate that it is safe to turn off
// power.
* Some hubs have a quirk where VCC can't be turned off without causing the
* hub to immediately power back on. The return value indicates if it is safe
* to attempt to power off the hub or not.
// quirk is not needed when button is pressed - button is active low
// Some hubs will turn themselves back on if I/O port VCC is off when
// we call pbdrv_reset_power_off(). So don't turn off power until the
// return value of pbdrv_ioport_power_off() tells us it is safe to do
// so.
Is it correct to summarize as follows?
- Inventor/Prime/Essential/Technic: Power off VCC any time before reset, then power off mcu (unless charging).
- City/Move Hub: Power off VCC when shutting down while button pressed, then power VCC back on and then power off mcu.
I am understanding correctly that the following quirk should no longer be used anywhere?
No, it is still needed. It is implemented here:
bool pbdrv_ioport_power_off(void) {
init_ports();
// Turn off power on pin 4 on all ports.
#if PBDRV_CONFIG_IOPORT_PUP_QUIRK_SHUTDOWN
if (pbdrv_ioport_needs_shutdown_quirk()) {
// Some hubs will turn themselves back on if VCC is off when power is
// turned off, so we have to turn VCC back on as a workaround.
pbdrv_ioport_enable_vcc(true);
return false;
}
// We want VCC off so that lights on sensors turn off while the power
// button is held down. But, this would cause the hub to turn back on
// immediately. So return true to indicate that it is safe to turn off
// power.
pbdrv_ioport_enable_vcc(false);
return true;
#else
pbdrv_ioport_enable_vcc(false);
return false;
#endif
}
- Inventor/Prime/Essential/Technic: Power off VCC any time before reset, then reset (unless charging).
Probably just wrong choice of words here, but I consider reset a different operation from power off. For reset, we just reset and don't touch power. For power off, we just poweroff and don't reset.
- City/Move Hub: Power off VCC when shutting down while button pressed, then power VCC back on and then reset.
See code above. There is no "reset" the hub just powers off because the button is released.
I am understanding correctly that the following quirk should no longer be used anywhere?
// Turn off power on pin 4 on all ports. This is set to input instead of // low to avoid city/move hubs turning back on when button released. // as soon as the user releases the power button pbdrv_gpio_input(&pbdrv_ioport_pup_platform_data.port_vcc);No, it is still needed. It is implemented here:
I don't see the pin being set to input anywhere anymore?
Probably just wrong choice of words here, but I consider reset a different operation from power off.
The method is called pdrv_reset_poweroff and I just abbreviated it the wrong way in my note above. It is now adjusted.
I implemented it now with the flow in the screenshot above here. I'll need to find a Move Hub that had this problem to confirm though. I'm wondering if with this approach there is enough time between turning VCC back on and powering off that the quirk still works.
I don't see the pin being set to input anywhere anymore?
Ah, it doesn't need to be set to input, pbdrv_ioport_enable_vcc(true); has the same effect.
. I'm wondering if with this approach there is enough time between turning VCC back on and powering off that the quirk still works.
Seems like it should work the same as what I had done. I can test it this weekend.
Seems like it should work the same as what I had done. I can test it this weekend.
Done. Tested both movehub and cityhub. Confirmed problem exists before commit and is fixed after commit.
Inspired by this PR, similar changes have been implemented in https://github.com/pybricks/pybricks-micropython/commit/77c5ed3f7cff638df5521ba7d59978fef57b6913 and simplified in https://github.com/pybricks/pybricks-micropython/commit/78006dfc by taking platform-specifics into the respective platform.c files.