Various minor enhancements for the 'console' command #1571
Conversation
sjg20
force-pushed
the
push4-cons
branch
3 times, most recently
from
6e98f86 to
a8b218b
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #1571 +/- ##
========================================
- Coverage 55.6% 55.3% -0.4%
========================================
Files 172 173 +1
Lines 13471 13604 +133
========================================
+ Hits 7500 7530 +30
- Misses 5971 6074 +103
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
labgrid/remote/client.py
Outdated
| @@ -988,7 +988,9 @@ async def _console(self, place, target, timeout, *, logfile=None, loop=False, li | |||
| if logfile: | |||
| logging.warning("--logfile option not supported by telnet, ignoring") | |||
|
|
|||
| print(f"connecting to {resource} calling {' '.join(call)}") | |||
| if logfile: | |||
| call.append(f"--logfile={logfile}") | |||
There was a problem hiding this comment.
Maybe this should be added to the commit message?
| @@ -1807,6 +1809,9 @@ def main(): | |||
| subparser.set_defaults(func=ClientSession.acquire) | |||
|
|
|||
| subparser = subparsers.add_parser("release", aliases=("unlock",), help="release a place") | |||
| subparser.add_argument( | |||
| "-a", "--auto", action="store_true", help="don't raise an error if the place is not acquired" | |||
| ) | |||
There was a problem hiding this comment.
Earlier -a is used as acquire, wouldn't be a -f force option here be less confusing?
|
Thanks for these patches, I applied most of them to my staging tree for testing in my lab. Some patches no longer apply against the master branch, could you kindly rebase this? Regarding the internal console, does it make sense to make it more configurable so it's used for i.e. |
It is common to acquire a place and then use it to access the console. Add a -a option to support this, to avoid running labgrid-client twice, which takes longer and makes error handling harder. If the place is acquired, it is released on exit. Signed-off-by: Simon Glass <[email protected]>
Some messages are printed regardless of the logging level. Convert them to logging statements so they can be suppressed. Signed-off-by: Simon Glass <[email protected]>
When a command fails in labgrid-client it just shows the return code. This makes it very hard to see what actually went wrong. Handle this exception explicitly, showing the output of the tool which failed. This allows easy debugging of the problem. This is particularly important with things like USB loaders. Signed-off-by: Simon Glass <[email protected]>
When a target may or may not be acquired, it is useful to be able to release it without checking first. Add an option for this, so that it is not necessary to check first. Signed-off-by: Simon Glass <[email protected]>
Use logging rather than print so that this output can be controlled. Signed-off-by: Simon Glass <[email protected]>
At present if a console device is used by a strategy driver it is then removed by the client immediately afterwards. If the 'console' command is used, this means that any console data is lost. This is done so that the console device is release for microcom, so move the logic in with the microcom logic. With the forthcoming internal terminal, it will not be needed. Signed-off-by: Simon Glass <[email protected]>
Put this code into a function to reduce the size of the _get_target() function. Signed-off-by: Simon Glass <[email protected]>
It is possible to set the initial state of a strategy, but not the final state. In many cases we want to power the board off and otherwise clean things up at the end. Provide a -e option to accomplish this. Signed-off-by: Simon Glass <[email protected]>
It is useful to be able to see if a place is allowed without raising an error and needing a try...except block. Add a function to handle this and update _check_allowed() to use it. This will be used by the updated terminal suport. Signed-off-by: Simon Glass <[email protected]>
There is quite a lot of code here, so move the terminal function into its own file. This will make it easier to extend it later. Signed-off-by: Simon Glass <[email protected]>
At present Labgrid uses microcom as its console. This has some limitations: - console output is lost between when the board is reset and microcom connects - txdelay cannot be handled in microcom, meaning that boards may fail to receive expected output - the console may echo a few characters back to the caller in the time between when 'labgrid-client console' is executed and when microcom starts (which causes failures with U-Boot test system) For many use cases, microcom is more than is needed, so provide a simple internal terminal which resolved the above problems. It is enabled by a '-i' option to the 'console' command, as well as an environment variable, so that it can be adjustly without updating a lot of scripts. To exit, press Ctrl-] twice, quickly. Series-changes: 4 - Get internal console working with qemu - Show a prompt when starting, to indicate it is waiting for the board Signed-off-by: Simon Glass <[email protected]>
Yes, done. Thanks for looking at it.
There is a LG_CONSOLE variable which I set in the pytest setup. Do you have any suggestions on how to make this work for you? |
Sorry I missed that, will apply it to my setup and test further. Thanks |
Provides an internal console and a way to automatically select a final state
The internal console avoids the problem of losing output when labgrid closes the serial port and re-opens it in the switch from to microcom.
The final state can be used to ensure that the board is off after use, without needing to run labgrid-client again. It is similar to the initial state in that sense.
Together, this PR allows 'labgrid client' to function as a console device suitable for use with U-Boot's test.py, for example
See here for how this is used in a real lab:
https://ci.u-boot.org/u-boot/u-boot/-/jobs/19119
Checklist