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

[Bug] LPF2 connection manager not ready before script starts #674

Closed
laurensvalk opened this issue Jun 22, 2022 · 3 comments
Closed

[Bug] LPF2 connection manager not ready before script starts #674

laurensvalk opened this issue Jun 22, 2022 · 3 comments
Labels
bug Something isn't working platform: Powered Up Issues related to LEGO Powered Up software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: motors Issues involving motors topic: sensors Issues involving sensors

Comments

@laurensvalk
Copy link
Member

This is a follow up from #673, but this is a different issue:

When a script is included with the firmware and you start it with the button, it often fails because the devices are not ready yet.

We are already waiting on UART devices to sync up (solved in #673), but there is a step before that as well to detect the device type. This takes about a second. There's a Python script below to reproduce it.

Either this step does not work right immediately after switching on the power to the ports, or it hasn't asynchronously initialized this driver yet by the time the user starts the program.

Perhaps we can wait for all drivers to be ready before allowing the user program to start. We could use a bootup animation to indicate that it is getting ready.

To support this hypothesis, try this hack which works around the problem:

diff --git a/lib/pbio/drv/ioport/ioport_lpf2.c b/lib/pbio/drv/ioport/ioport_lpf2.c
index c01aa1e6..07efb985 100644
--- a/lib/pbio/drv/ioport/ioport_lpf2.c
+++ b/lib/pbio/drv/ioport/ioport_lpf2.c
@@ -188,12 +188,18 @@ static void init_one(uint8_t ioport) {
     basic_devs[ioport].ops = &basic_dev_ops;
 }
 
+static uint32_t dcm_wait_remaining = 500;
+
 // TODO: This should be moved to a common ioport_core.c file or removed entirely
 pbio_error_t pbdrv_ioport_get_iodev(pbio_port_id_t port, pbio_iodev_t **iodev) {
     if (port < PBDRV_CONFIG_IOPORT_LPF2_FIRST_PORT || port > PBDRV_CONFIG_IOPORT_LPF2_LAST_PORT) {
         return PBIO_ERROR_INVALID_PORT;
     }
 
+    if (dcm_wait_remaining) {
+        return PBIO_ERROR_AGAIN;
+    }
+
     *iodev = ioport_devs[port - PBDRV_CONFIG_IOPORT_LPF2_FIRST_PORT].iodev;
     if (*iodev == NULL) {
         return PBIO_ERROR_NO_DEV;
@@ -513,6 +519,9 @@ PROCESS_THREAD(pbdrv_ioport_lpf2_process, ev, data) {
                     }
                 }
             }
+            if (dcm_wait_remaining) {
+                dcm_wait_remaining--;
+            }
         }
     }

To reproduce

Install this script along with the firmware and attach a motor to port A.

from pybricks.hubs import TechnicHub
from pybricks.pupdevices import Motor
from pybricks.parameters import Color, Port
from pybricks.tools import wait

hub = TechnicHub()

hub.light.on(Color.MAGENTA)

try:
    motor = Motor(Port.A)
    hub.light.on(Color.GREEN)
except OSError:
    hub.light.on(Color.RED)

wait(10 * 1000)

Without the patch, the light turns red if you start this right after boot.

With the patch, it turns magenta and then green once the device is ready.

This is tested on Technic Hub. We don't see this on Prime Hub, but that's probably only because there's already a wait for flash to get ready before the user can start the program.

@laurensvalk laurensvalk added platform: Powered Up Issues related to LEGO Powered Up topic: motors Issues involving motors topic: sensors Issues involving sensors bug Something isn't working labels Jun 22, 2022
@dlech
Copy link
Member

dlech commented Jun 22, 2022

Perhaps we can wait for all drivers to be ready before allowing the user program to start.

We already do this. Although that doesn't rule out a bug in one of the drivers that doesn't properly indicate that it is still busy.

We could use a bootup animation to indicate that it is getting ready.

We can't do anything until all the drivers are ready since the light may be one of the drivers that isn't ready. And the fact that the light already comes on almost instantly (it already waits for all drivers to be ready) means that we wouldn't have time to see this indication anyway. We could do some kind of indication any time sensors are syncing up though (not just at boot).

We should also figure out why the high baud rate sync isn't working and fix it.

@laurensvalk
Copy link
Member Author

We already do this. Although that doesn't rule out a bug in one of the drivers that doesn't properly indicate that it is still busy.

The driver might be ready by our current definition, but maybe it takes a while for the DCM to start giving the right results.

@laurensvalk
Copy link
Member Author

For this release, I'd be OK with a cleaned up version of the patch above.

@dlech dlech added the software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) label Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working platform: Powered Up Issues related to LEGO Powered Up software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: motors Issues involving motors topic: sensors Issues involving sensors
Projects
None yet
Development

No branches or pull requests

2 participants