| Message ID | 20240828220846.1205813-18-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | labgrid: Provide an integration with Labgrid | expand |
On 29/08/2024 00:08, Simon Glass wrote: > Send the Labgrid quit characters to ask it to exit gracefully. This > typically allows it to power off the board being used. Sending those characters every time could collide with other CI systems, I don't think it's a good idea. > > If that doesn't work, try the less graceful approach. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > test/py/u_boot_spawn.py | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py > index c0ff0813554..ec1fa465047 100644 > --- a/test/py/u_boot_spawn.py > +++ b/test/py/u_boot_spawn.py > @@ -16,6 +16,9 @@ import termios > import time > import traceback > > +# Character to send (twice) to exit the terminal > +EXIT_CHAR = 0x1d # FS (Ctrl + ]) > + > class Timeout(Exception): > """An exception sub-class that indicates that a timeout occurred.""" > > @@ -304,15 +307,25 @@ class Spawn: > None. > > Returns: > - Nothing. > + str: Type of closure completed > """ > + self.send(chr(EXIT_CHAR) * 2) > > + # Wait about 10 seconds for Labgrid to close and power off the board > + for _ in range(100): > + if not self.isalive(): > + return 'normal' > + time.sleep(0.1) > + > + # That didn't work, so try closing the PTY > os.close(self.fd) > for _ in range(100): > if not self.isalive(): > - break > + return 'break' > time.sleep(0.1) > > + return 'timeout' > + > def get_expect_output(self): > """Return the output read by expect() >
Hi Neil, On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > On 29/08/2024 00:08, Simon Glass wrote: > > Send the Labgrid quit characters to ask it to exit gracefully. This > > typically allows it to power off the board being used. > > Sending those characters every time could collide with other CI systems, > I don't think it's a good idea. What systems are you thinking about and what sort of collision would occur? What do you suggest instead? > > > > > If that doesn't work, try the less graceful approach. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > (no changes since v1) > > > > test/py/u_boot_spawn.py | 17 +++++++++++++++-- > > 1 file changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py > > index c0ff0813554..ec1fa465047 100644 > > --- a/test/py/u_boot_spawn.py > > +++ b/test/py/u_boot_spawn.py > > @@ -16,6 +16,9 @@ import termios > > import time > > import traceback > > > > +# Character to send (twice) to exit the terminal > > +EXIT_CHAR = 0x1d # FS (Ctrl + ]) > > + > > class Timeout(Exception): > > """An exception sub-class that indicates that a timeout occurred.""" > > > > @@ -304,15 +307,25 @@ class Spawn: > > None. > > > > Returns: > > - Nothing. > > + str: Type of closure completed > > """ > > + self.send(chr(EXIT_CHAR) * 2) > > > > + # Wait about 10 seconds for Labgrid to close and power off the board > > + for _ in range(100): > > + if not self.isalive(): > > + return 'normal' > > + time.sleep(0.1) > > + > > + # That didn't work, so try closing the PTY > > os.close(self.fd) > > for _ in range(100): > > if not self.isalive(): > > - break > > + return 'break' > > time.sleep(0.1) > > > > + return 'timeout' > > + > > def get_expect_output(self): > > """Return the output read by expect() > > > Regards, Simon
On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > Hi Neil, > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > typically allows it to power off the board being used. > > > > Sending those characters every time could collide with other CI systems, > > I don't think it's a good idea. > > What systems are you thinking about and what sort of collision would occur? > > What do you suggest instead? Why do we need this at all? Did I miss where we send picocom the disconnect nicely key-combination?
Hi Tom, On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > Hi Neil, > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > typically allows it to power off the board being used. > > > > > > Sending those characters every time could collide with other CI systems, > > > I don't think it's a good idea. > > > > What systems are you thinking about and what sort of collision would occur? > > > > What do you suggest instead? > > Why do we need this at all? Did I miss where we send picocom the > disconnect nicely key-combination? When labgrid gets a signal, it exits. It doesn't continue its co-routines and execute the end strategy to power things off, etc. I suspect it could be made to do that, but I already have 62 Labgrid patches, so I thought this would be expedient... I can make this conditional on the new USE_LABGRID variable. Regards, Simon
On Thu, Aug 29, 2024 at 07:04:36PM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > > Hi Neil, > > > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > > typically allows it to power off the board being used. > > > > > > > > Sending those characters every time could collide with other CI systems, > > > > I don't think it's a good idea. > > > > > > What systems are you thinking about and what sort of collision would occur? > > > > > > What do you suggest instead? > > > > Why do we need this at all? Did I miss where we send picocom the > > disconnect nicely key-combination? > > When labgrid gets a signal, it exits. It doesn't continue its > co-routines and execute the end strategy to power things off, etc. I > suspect it could be made to do that, but I already have 62 Labgrid > patches, so I thought this would be expedient... > > I can make this conditional on the new USE_LABGRID variable. It sounds to me like we need to make generic improvements to our hooks then, if there's not a "now call poweroff" hook.
Hi Tom, On Fri, 30 Aug 2024 at 08:26, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Aug 29, 2024 at 07:04:36PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > > > Hi Neil, > > > > > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > > > typically allows it to power off the board being used. > > > > > > > > > > Sending those characters every time could collide with other CI systems, > > > > > I don't think it's a good idea. > > > > > > > > What systems are you thinking about and what sort of collision would occur? > > > > > > > > What do you suggest instead? > > > > > > Why do we need this at all? Did I miss where we send picocom the > > > disconnect nicely key-combination? > > > > When labgrid gets a signal, it exits. It doesn't continue its > > co-routines and execute the end strategy to power things off, etc. I > > suspect it could be made to do that, but I already have 62 Labgrid > > patches, so I thought this would be expedient... > > > > I can make this conditional on the new USE_LABGRID variable. > > It sounds to me like we need to make generic improvements to our hooks > then, if there's not a "now call poweroff" hook. The thing is, Labgrid has its own internal console, which allows me to see all the output from reset. If I use picocom or some other program then some of the output is gone by the time I connect, particularly when using USB download. Because of that, just killing Labgrid, which is what pytest does, is a pretty heavy hammer and leaves things in an unknown state. So I added this method to give it a software signal. Regards, Simon
On Sun, Sep 01, 2024 at 02:09:58PM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 30 Aug 2024 at 08:26, Tom Rini <trini@konsulko.com> wrote: > > > > On Thu, Aug 29, 2024 at 07:04:36PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > > > > Hi Neil, > > > > > > > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > > > > typically allows it to power off the board being used. > > > > > > > > > > > > Sending those characters every time could collide with other CI systems, > > > > > > I don't think it's a good idea. > > > > > > > > > > What systems are you thinking about and what sort of collision would occur? > > > > > > > > > > What do you suggest instead? > > > > > > > > Why do we need this at all? Did I miss where we send picocom the > > > > disconnect nicely key-combination? > > > > > > When labgrid gets a signal, it exits. It doesn't continue its > > > co-routines and execute the end strategy to power things off, etc. I > > > suspect it could be made to do that, but I already have 62 Labgrid > > > patches, so I thought this would be expedient... > > > > > > I can make this conditional on the new USE_LABGRID variable. > > > > It sounds to me like we need to make generic improvements to our hooks > > then, if there's not a "now call poweroff" hook. > > The thing is, Labgrid has its own internal console, which allows me to > see all the output from reset. If I use picocom or some other program > then some of the output is gone by the time I connect, particularly > when using USB download. Because of that, just killing Labgrid, which > is what pytest does, is a pretty heavy hammer and leaves things in an > unknown state. So I added this method to give it a software signal. OK, but we shouldn't care? You can have the console available to pytest and a terminal at the same time, with labgrid. If you need to see before pytest grabs it, have the terminal be the first console, not pytest? This gets back again I think to your specific way of making use of labgrid contrasting with just generally supporting labgrid with however another physical lab has set it up. Does killing one connection really reset all of them? Or was it just a conflict with your auto-acquire/release?
Hi Tom, On Tue, 3 Sept 2024 at 12:48, Tom Rini <trini@konsulko.com> wrote: > > On Sun, Sep 01, 2024 at 02:09:58PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 30 Aug 2024 at 08:26, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Thu, Aug 29, 2024 at 07:04:36PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > > > > > Hi Neil, > > > > > > > > > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > > > > > typically allows it to power off the board being used. > > > > > > > > > > > > > > Sending those characters every time could collide with other CI systems, > > > > > > > I don't think it's a good idea. > > > > > > > > > > > > What systems are you thinking about and what sort of collision would occur? > > > > > > > > > > > > What do you suggest instead? > > > > > > > > > > Why do we need this at all? Did I miss where we send picocom the > > > > > disconnect nicely key-combination? > > > > > > > > When labgrid gets a signal, it exits. It doesn't continue its > > > > co-routines and execute the end strategy to power things off, etc. I > > > > suspect it could be made to do that, but I already have 62 Labgrid > > > > patches, so I thought this would be expedient... > > > > > > > > I can make this conditional on the new USE_LABGRID variable. > > > > > > It sounds to me like we need to make generic improvements to our hooks > > > then, if there's not a "now call poweroff" hook. > > > > The thing is, Labgrid has its own internal console, which allows me to > > see all the output from reset. If I use picocom or some other program > > then some of the output is gone by the time I connect, particularly > > when using USB download. Because of that, just killing Labgrid, which > > is what pytest does, is a pretty heavy hammer and leaves things in an > > unknown state. So I added this method to give it a software signal. > > OK, but we shouldn't care? You can have the console available to pytest > and a terminal at the same time, with labgrid. If you need to see before > pytest grabs it, have the terminal be the first console, not pytest? I'm a bit lost at this point...'labgrid-client console' creates a console connection and it keeps running until killed, with that console connection connected between its stdin/stdout. Then pytest uses that. There is only one console. I care because missing output makes it impossible to see what went wrong, if something went wrong. > This gets back again I think to your specific way of making use of > labgrid contrasting with just generally supporting labgrid with however > another physical lab has set it up. Does killing one connection really > reset all of them? Or was it just a conflict with your > auto-acquire/release? Are you talking about the Labgrid exporter, perhaps? This patch is for the client and we have one client process running for each board we connect to. You can see that in the Labgrid version of the u-boot-test-console script which basically runs labgrid-client and let's it do the talking. Regards, Simon
On Thu, Sep 05, 2024 at 06:50:20PM -0600, Simon Glass wrote: > Hi Tom, > > On Tue, 3 Sept 2024 at 12:48, Tom Rini <trini@konsulko.com> wrote: > > > > On Sun, Sep 01, 2024 at 02:09:58PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 30 Aug 2024 at 08:26, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Thu, Aug 29, 2024 at 07:04:36PM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > > > > > > Hi Neil, > > > > > > > > > > > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > > > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > > > > > > typically allows it to power off the board being used. > > > > > > > > > > > > > > > > Sending those characters every time could collide with other CI systems, > > > > > > > > I don't think it's a good idea. > > > > > > > > > > > > > > What systems are you thinking about and what sort of collision would occur? > > > > > > > > > > > > > > What do you suggest instead? > > > > > > > > > > > > Why do we need this at all? Did I miss where we send picocom the > > > > > > disconnect nicely key-combination? > > > > > > > > > > When labgrid gets a signal, it exits. It doesn't continue its > > > > > co-routines and execute the end strategy to power things off, etc. I > > > > > suspect it could be made to do that, but I already have 62 Labgrid > > > > > patches, so I thought this would be expedient... > > > > > > > > > > I can make this conditional on the new USE_LABGRID variable. > > > > > > > > It sounds to me like we need to make generic improvements to our hooks > > > > then, if there's not a "now call poweroff" hook. > > > > > > The thing is, Labgrid has its own internal console, which allows me to > > > see all the output from reset. If I use picocom or some other program > > > then some of the output is gone by the time I connect, particularly > > > when using USB download. Because of that, just killing Labgrid, which > > > is what pytest does, is a pretty heavy hammer and leaves things in an > > > unknown state. So I added this method to give it a software signal. > > > > OK, but we shouldn't care? You can have the console available to pytest > > and a terminal at the same time, with labgrid. If you need to see before > > pytest grabs it, have the terminal be the first console, not pytest? > > I'm a bit lost at this point...'labgrid-client console' creates a > console connection and it keeps running until killed, with that > console connection connected between its stdin/stdout. Then pytest > uses that. > > There is only one console. > > I care because missing output makes it impossible to see what went > wrong, if something went wrong. There's not one console, is what I'm saying. You can connect to the console via a terminal while pytest is running from another terminal. So if you have a problem and it's in that window where we're just starting up, grab the console for you first and then let pytest go second. > > This gets back again I think to your specific way of making use of > > labgrid contrasting with just generally supporting labgrid with however > > another physical lab has set it up. Does killing one connection really > > reset all of them? Or was it just a conflict with your > > auto-acquire/release? > > Are you talking about the Labgrid exporter, perhaps? This patch is for > the client and we have one client process running for each board we > connect to. You can see that in the Labgrid version of the > u-boot-test-console script which basically runs labgrid-client and > let's it do the talking. It's about the assumptions that are true for your lab (and possibly others) that aren't true for my lab (and possibly others).
Hi Tom, On Fri, 6 Sept 2024 at 12:43, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Sep 05, 2024 at 06:50:20PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 3 Sept 2024 at 12:48, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Sun, Sep 01, 2024 at 02:09:58PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Fri, 30 Aug 2024 at 08:26, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Thu, Aug 29, 2024 at 07:04:36PM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Thu, 29 Aug 2024 at 10:59, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > > > On Thu, Aug 29, 2024 at 09:01:17AM -0600, Simon Glass wrote: > > > > > > > > Hi Neil, > > > > > > > > > > > > > > > > On Thu, 29 Aug 2024 at 08:26, <neil.armstrong@linaro.org> wrote: > > > > > > > > > > > > > > > > > > On 29/08/2024 00:08, Simon Glass wrote: > > > > > > > > > > Send the Labgrid quit characters to ask it to exit gracefully. This > > > > > > > > > > typically allows it to power off the board being used. > > > > > > > > > > > > > > > > > > Sending those characters every time could collide with other CI systems, > > > > > > > > > I don't think it's a good idea. > > > > > > > > > > > > > > > > What systems are you thinking about and what sort of collision would occur? > > > > > > > > > > > > > > > > What do you suggest instead? > > > > > > > > > > > > > > Why do we need this at all? Did I miss where we send picocom the > > > > > > > disconnect nicely key-combination? > > > > > > > > > > > > When labgrid gets a signal, it exits. It doesn't continue its > > > > > > co-routines and execute the end strategy to power things off, etc. I > > > > > > suspect it could be made to do that, but I already have 62 Labgrid > > > > > > patches, so I thought this would be expedient... > > > > > > > > > > > > I can make this conditional on the new USE_LABGRID variable. > > > > > > > > > > It sounds to me like we need to make generic improvements to our hooks > > > > > then, if there's not a "now call poweroff" hook. > > > > > > > > The thing is, Labgrid has its own internal console, which allows me to > > > > see all the output from reset. If I use picocom or some other program > > > > then some of the output is gone by the time I connect, particularly > > > > when using USB download. Because of that, just killing Labgrid, which > > > > is what pytest does, is a pretty heavy hammer and leaves things in an > > > > unknown state. So I added this method to give it a software signal. > > > > > > OK, but we shouldn't care? You can have the console available to pytest > > > and a terminal at the same time, with labgrid. If you need to see before > > > pytest grabs it, have the terminal be the first console, not pytest? > > > > I'm a bit lost at this point...'labgrid-client console' creates a > > console connection and it keeps running until killed, with that > > console connection connected between its stdin/stdout. Then pytest > > uses that. > > > > There is only one console. > > > > I care because missing output makes it impossible to see what went > > wrong, if something went wrong. > > There's not one console, is what I'm saying. You can connect to the > console via a terminal while pytest is running from another terminal. So > if you have a problem and it's in that window where we're just starting > up, grab the console for you first and then let pytest go second. This patch is actually about gracefully closing down labgrid-client. When pytest is running tests on a board, I don't need/want to connect to it separately. Note that with labgrid I added an internal terminal, to avoid picocom (or microcom) losing the initial output. I can't really say it any other way. It definitely happens and it is definitely a problem, promise! This patch is about letting labgrid shut down nicely, rather than leave things in a strange state, requiring a separate call to power things down, for example. > > > > This gets back again I think to your specific way of making use of > > > labgrid contrasting with just generally supporting labgrid with however > > > another physical lab has set it up. Does killing one connection really > > > reset all of them? Or was it just a conflict with your > > > auto-acquire/release? > > > > Are you talking about the Labgrid exporter, perhaps? This patch is for > > the client and we have one client process running for each board we > > connect to. You can see that in the Labgrid version of the > > u-boot-test-console script which basically runs labgrid-client and > > let's it do the talking. > > It's about the assumptions that are true for your lab (and possibly > others) that aren't true for my lab (and possibly others). OK. I'm very happy with my lab at the moment. It permits pytest, interactive use and gitlab. But, yes, there are many ways to do all this... Regards, Simon
diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index c0ff0813554..ec1fa465047 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -16,6 +16,9 @@ import termios import time import traceback +# Character to send (twice) to exit the terminal +EXIT_CHAR = 0x1d # FS (Ctrl + ]) + class Timeout(Exception): """An exception sub-class that indicates that a timeout occurred.""" @@ -304,15 +307,25 @@ class Spawn: None. Returns: - Nothing. + str: Type of closure completed """ + self.send(chr(EXIT_CHAR) * 2) + # Wait about 10 seconds for Labgrid to close and power off the board + for _ in range(100): + if not self.isalive(): + return 'normal' + time.sleep(0.1) + + # That didn't work, so try closing the PTY os.close(self.fd) for _ in range(100): if not self.isalive(): - break + return 'break' time.sleep(0.1) + return 'timeout' + def get_expect_output(self): """Return the output read by expect()
Send the Labgrid quit characters to ask it to exit gracefully. This typically allows it to power off the board being used. If that doesn't work, try the less graceful approach. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) test/py/u_boot_spawn.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)