Skip to content

pyterm: Add device timeout#12270

Closed
chrysn wants to merge 1 commit intoRIOT-OS:masterfrom
chrysn-pull-requests:pyterm-device-timeout
Closed

pyterm: Add device timeout#12270
chrysn wants to merge 1 commit intoRIOT-OS:masterfrom
chrysn-pull-requests:pyterm-device-timeout

Conversation

@chrysn
Copy link
Member

@chrysn chrysn commented Sep 18, 2019

Contribution description

When pyterm is used on boards that use CDC-ACM stdio (ie. provide their own USB device), that device is not available immediately after flashing, and a make flash term is likely to fail.

This adds a --device-timeout option to pyterm that will repeat pyterm's device opening step until the timeout is exceeded (in which case it will fail as it had before), or the device can be opened.

Boards that use such a configuration will want to set PYTERMFLAGS += --device-timeout=5 or another suitable value depending on the board's expected time to go from bootloader to operational.

Testing procedure

On a board that uses the the stdio_cdc_acm module, set PYTERMFLAGS=--device-timeout=5 and make flash term and see that the terminal opens after a few seconds rather than failing with a "device not found" message.

On other boards the test can be emulated by running make term with the serial connection unplugged, plugging it in a second after the command has started.

Issues/PRs references

The issue will become more widespread when #11085 is available and boards like the nrf52840-dongle (#12189) become more easily usable with RIOT.

Alternatives

This solves the problem only for pyterm. Worse yet, it may still be racy if the bootloader used is occupying the same file name as the serial target. (That is the case on my board, but the race always went well, presumably because the USB reset caused by the flashing tool is immediate).

An alternative solution would be to have a term-ready target that term would always depend on, and boards could set a TERM_READY_TOOL (defaulting to true?) that would be active in that stage. A simple tool that does not handle the racing case could be shipped as a shell script. The variable would also provide an entry point for a more sophisticated solution that can tell boot loaders apart from real terminal ttys.

I could implement that as well (come to think of it writing this last line I'm leaning more and more towards that rather than this PR), but would need some guidance in the complex Makefile ecosystem.

When pyterm is used on boards that use CDC-ACM stdio (ie. provide their
own USB device), that device is not available immediately after
flashing, and a `make flash term` is likely to fail.

This adds a --device-timeout option to pyterm that will repeat pyterm's
device opening step until the timeout is exceeded (in which case it will
fail as it had before), or the device can be opened.

Boards that use such a configuration will want to set
PYTERMFLAGS += --device-timeout=5 or another suitable value depending on
the board's expected time to go from bootloader to operational.
@miri64
Copy link
Member

miri64 commented Sep 18, 2019

When pyterm is used on boards that use CDC-ACM stdio (ie. provide their own USB device), that device is not available immediately after flashing, […]

Does this also apply to resetting? If yes, this might become a problem with the make test if the MAKE_TERM_STARTED_DELAY environment variable isn't set properly.

# on many platforms, the termprog needs a short while to be ready...
time.sleep(MAKE_TERM_STARTED_DELAY)

This solves the problem only for pyterm. Worse yet, it may still be racy if the bootloader used is occupying the same file name as the serial target. (That is the case on my board, but the race always went well, presumably because the USB reset caused by the flashing tool is immediate).

Given these problems, and the one I pointed out, I think I prefer the alternative you proposed. What does @cladmi think?

@cladmi
Copy link
Contributor

cladmi commented Sep 18, 2019

Question, with #11085, does not the terminal disappear after the make reset ? Because that would cause many issues with the current state.
I want to get rid of make reset while make term is running as it is not always available and breaks on some boards. https://git.imp.fu-berlin.de/riot-appstore/riot-buildsystem-ci-server/blob/46ecfdb7e44b0e54832e0135bac46fe8638b6cf8/tools/makefiles.pre#L79-82

But it will need some migration to do an interactive synchronization (or any synchronization) with the test before dumping the output as for example: f193ffd

Handling of the term not being ready

I would be somehow more for a term-ready, term-available target, to handle these issues of not being able to talk to the board before.
By default, it would do nothing, but on board that needs this 'tty' being not busy, would wait for it.
If the tty is ready, it would need to return "immediately" though, not sleep 5 seconds just in case.

Which would make that when you do make term, it waits until the device is there.

And in test, it could call term-ready before, wait until it returns, then start the test procedure with its MAKE_TERM_STARTED_DELAY thing.

This way, no issue with breaking the test due to the reconnect time not taken into account or giving a wrong failure reason:

#10482 (comment)

@chrysn
Copy link
Member Author

chrysn commented Sep 18, 2019

does not the terminal disappear after the make reset
The terminal will disappear on reset.

Whether make reset can be implemented depens on the programmer. For the nrfutil programmer I'm using this with (that uses the factory provided nRF52840 bootloader), there is simply no viable make reset (unless the device implements a shell and offers a reboot command with which make reset can be implemented; in the programmers' ecosystem I think they use some bespoke USB reset-and-enter-bootloader command).

OK, I'll shut this down then and try again with a term-ready target, aiming for usability with the term target first, and from there we can explore the applicability to testing.

@chrysn chrysn closed this Sep 18, 2019
@cladmi
Copy link
Contributor

cladmi commented Sep 19, 2019

The terminal will disappear on reset.

That is what I expected.

Whether make reset can be implemented depens on the programmer. For the nrfutil programmer I'm using this with (that uses the factory provided nRF52840 bootloader), there is simply no viable make reset (unless the device implements a shell and offers a reboot command with which make reset can be implemented; in the programmers' ecosystem I think they use some bespoke USB reset-and-enter-bootloader command).

Agreed. That is why I wanted to change the testing environment to not do reset anymore (or at least, not after term), but did not manage to find the time to carry the migration.

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

Successfully merging this pull request may close these issues.

3 participants