diff mbox series

Sharing a hardware lab

Message ID dbf4c70f7dc3992c0603318e47b262175fa66cc4.camel@denx.de
State New
Headers show
Series Sharing a hardware lab | expand

Commit Message

Harald Seiler March 22, 2020, 9:55 a.m. UTC
Hi Simon,

On Sat, 2020-03-21 at 13:07 -0600, Simon Glass wrote:
> Hi Harald,
> 
> On Mon, 24 Feb 2020 at 06:27, Harald Seiler <hws at denx.de> wrote:
> > Hello Simon,
> > 
> > On Sun, 2020-02-23 at 19:34 -0700, Simon Glass wrote:
> > > Hi Heiko,
> > > 
> > > Thanks for the hints! I pushed your things here:
> > > 
> > > https://github.com/sglass68/tbot/tree/simon
> > > 
> > > Then I try:
> > > tbot -l kea.py -b pcduino3.py uboot_build
> > > 
> > > and get an error:
> > > 
> > > tbot starting ...
> > > type <class 'module'>
> > > ??Calling uboot_build ...
> > > ?   ??Fail. (0.000s)
> > > ??Exception:
> > > ?   Traceback (most recent call last):
> > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/main.py",
> > > line 318, in main
> > > ?       func(**params)
> > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/decorators.py",
> > > line 103, in wrapped
> > > ?       result = tc(*args, **kwargs)
> > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/tc/uboot/build.py",
> > > line 271, in _build
> > > ?       builder = UBootBuilder._get_selected_builder()
> > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/tc/uboot/build.py",
> > > line 160, in _get_selected_builder
> > > ?       builder = getattr(tbot.selectable.UBootMachine, "build")
> > > ?   AttributeError: type object 'UBootMachine' has no attribute 'build'
> > > 
> > > I'm a bit lost in all the classes and functions. A working example or
> > > a patch would be great!
> > > 
> > > I've pushed all my scripts here:
> > > 
> > > https://github.com/sglass68/ubtest
> > > 
> > > The top commit is your patches.
> > 
> > I think you mixed a few things up while adding Heiko's stuff.  Instead
> > of your last commit, attempt the attached patch.  It is untested of
> > course but should point you in the right direction; here is a short
> > summary of what it adds:
> > 
> >     1. Your `kea` lab needs to be marked as a build-host.  This means it
> >        needs the `toolchains` property which returns what toolchains are
> >        available.  I added a dummy armv7-a toolchain which might need
> >        a bit more adjustment to work for you.
> > 
> >     2. I created a UBootBuilder for pcduino3.  This builder just
> >        specifies what defconfig to build and what toolchain to use (need
> >        to be defined in the selected lab).
> > 
> >        Heiko's builder config is a bit more elaborate and also does some
> >        patching after applying the defconfig.  This is of course also
> >        possible if you want to.
> > 
> >     3. I added a U-Boot config for your board which is needed because
> >        its `build` property specifies which U-Boot builder to use.  This
> >        will make the `uboot_build` testcase work properly.  Later you
> >        might want to adjust this U-Boot config to actually work so you
> >        can make use of it for flashing the new U-Boot binary.
> > 
> > Some more links to documentation:
> > 
> >     - Build-host config: https://tbot.tools/modules/machine_linux.html#builder
> >     - UBootBuilder class: https://tbot.tools/modules/tc.html#tbot.tc.uboot.UBootBuilder
> > 
> > Hope this helps!
> 
> Yes it helps a lot, thank you. I now have it building U-Boot:
> 
>    tbot -l kea.py -b pcduino3.py -p clean=False uboot_build interactive_uboot
> 
> I sent a tiny patch to tbot to fix an error I had, and I made a few
> minor changes to what you sent.
> 
> https://github.com/sglass68/ubtest/commit/7da7a3794d505e970e0e21a9b6ed3a7e5f2f2190

Huh, actually I messed up in the patch I sent you.  Please apply the following
diff:


This will make everything work as intended and the patch you needed in tbot is
also no longer necessary (Passing a class instead of an instance will not
bind the method's `self` argument so you got an error from a weird place.
Maybe I should add a check against this kind of mistake ...).
 
> So my next question is how to load U-Boot onto the board. I can see
> methods for poweron/poweroff but not for actually writing to the
> board. Is there a built-in structure for this? I cannot find it.
> 
> I am hoping that the Pcduino3 class can define a method like
> 'load_uboot()' to write U-Boot to the board?

I added something similar to this in our DENX internal tbot configurations but
did not yet publish it anywhere (maybe I should add it to tbot_contrib?).  Just
for you, here is what I did:

1. In the board configs, each U-Boot class defines a `flash()` method:

    from tbot.tc import shell, git

    class P2020RdbUBoot(board.Connector, board.UBootShell):
        name = "p2020rdb-uboot"
        prompt = "=> "
        build = P2020RdbUBootBuilder()

        def flash(self, repo: git.GitRepository) -> None:
            self.env("autoload", "no")
            self.exec0("dhcp")
            self.env("serverip", "192.168.1.1")

            tftpdir = self.host.fsroot / "tftpboot" / "p2020rdb" / "tbot2"
            if not tftpdir.is_dir():
                self.host.exec0("mkdir", "-p", tftpdir)

            name = "u-boot-with-spl.bin"
            tbot.log.message(f"Fetching {name} ...")
            shell.copy(repo / name, tftpdir / name)

            addr = 0x10000000
            self.exec0("tftp", hex(addr), "p2020rdb/tbot2/" + name)
            size = int(self.env("filesize"), 16)
            tbot.log.message(f"Downoad succeeded:\n  U-Boot: {size} bytes")

            tbot.log.message("Flashing ...")
            self.exec0("nand", "device", "0")
            self.exec0("nand", "erase.spread", "0", hex(size))
            self.exec0("nand", "write", hex(addr), "0", hex(size))

   As you can see, it get the repository (as a GitRepository [1]) where
   U-Boot was built, passed as an argument, and copies the relevant
   binaries to a tftp directory on the lab-host (using shell.copy() [2]).
   Then it downloads and flashes U-Boot like you would manually.

   Alternatively, I could imagine the implementation connecting to
   a hardware debugger and uploading the binary to flash that way.

2. I defined a few testcases for flashing.  Namely, `uboot_build_and_flash`
   (build U-Boot and then flash the new version) and `uboot_bare_flash` (don't
   rebuild, just flash the artifacts from the last build).  These then use the
   board-specific U-Boot builder and flash method.

   I also added a feature where you can specify `-fsafe` on the tbot
   command-line.  This will, in case something goes wrong while flashing,
   drop you onto the interactive U-Boot command-line instead of powering
   off the board and reporting a failure.

   I uploaded the testcases here:

        https://gitlab.denx.de/snippets/10

[1]: http://tbot.tools/modules/tc.html#tbot.tc.git.GitRepository
[2]: http://tbot.tools/modules/tc.html#tbot.tc.shell.copy

Comments

Wolfgang Denk March 22, 2020, 11:10 a.m. UTC | #1
Dear Harald,

In message <dbf4c70f7dc3992c0603318e47b262175fa66cc4.camel at denx.de> you wrote:
>
> I added something similar to this in our DENX internal tbot configurations but
> did not yet publish it anywhere (maybe I should add it to tbot_contrib?).  Just

Yes, please do!


Best regards,

Wolfgang Denk
Simon Glass March 22, 2020, 6:42 p.m. UTC | #2
Hi Harald,

On Sun, 22 Mar 2020 at 03:56, Harald Seiler <hws at denx.de> wrote:
>
> Hi Simon,
>
> On Sat, 2020-03-21 at 13:07 -0600, Simon Glass wrote:
> > Hi Harald,
> >
> > On Mon, 24 Feb 2020 at 06:27, Harald Seiler <hws at denx.de> wrote:
> > > Hello Simon,
> > >
> > > On Sun, 2020-02-23 at 19:34 -0700, Simon Glass wrote:
> > > > Hi Heiko,
> > > >
> > > > Thanks for the hints! I pushed your things here:
> > > >
> > > > https://github.com/sglass68/tbot/tree/simon
> > > >
> > > > Then I try:
> > > > tbot -l kea.py -b pcduino3.py uboot_build
> > > >
> > > > and get an error:
> > > >
> > > > tbot starting ...
> > > > type <class 'module'>
> > > > ??Calling uboot_build ...
> > > > ?   ??Fail. (0.000s)
> > > > ??Exception:
> > > > ?   Traceback (most recent call last):
> > > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/main.py",
> > > > line 318, in main
> > > > ?       func(**params)
> > > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/decorators.py",
> > > > line 103, in wrapped
> > > > ?       result = tc(*args, **kwargs)
> > > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/tc/uboot/build.py",
> > > > line 271, in _build
> > > > ?       builder = UBootBuilder._get_selected_builder()
> > > > ?     File "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/tc/uboot/build.py",
> > > > line 160, in _get_selected_builder
> > > > ?       builder = getattr(tbot.selectable.UBootMachine, "build")
> > > > ?   AttributeError: type object 'UBootMachine' has no attribute 'build'
> > > >
> > > > I'm a bit lost in all the classes and functions. A working example or
> > > > a patch would be great!
> > > >
> > > > I've pushed all my scripts here:
> > > >
> > > > https://github.com/sglass68/ubtest
> > > >
> > > > The top commit is your patches.
> > >
> > > I think you mixed a few things up while adding Heiko's stuff.  Instead
> > > of your last commit, attempt the attached patch.  It is untested of
> > > course but should point you in the right direction; here is a short
> > > summary of what it adds:
> > >
> > >     1. Your `kea` lab needs to be marked as a build-host.  This means it
> > >        needs the `toolchains` property which returns what toolchains are
> > >        available.  I added a dummy armv7-a toolchain which might need
> > >        a bit more adjustment to work for you.
> > >
> > >     2. I created a UBootBuilder for pcduino3.  This builder just
> > >        specifies what defconfig to build and what toolchain to use (need
> > >        to be defined in the selected lab).
> > >
> > >        Heiko's builder config is a bit more elaborate and also does some
> > >        patching after applying the defconfig.  This is of course also
> > >        possible if you want to.
> > >
> > >     3. I added a U-Boot config for your board which is needed because
> > >        its `build` property specifies which U-Boot builder to use.  This
> > >        will make the `uboot_build` testcase work properly.  Later you
> > >        might want to adjust this U-Boot config to actually work so you
> > >        can make use of it for flashing the new U-Boot binary.
> > >
> > > Some more links to documentation:
> > >
> > >     - Build-host config: https://tbot.tools/modules/machine_linux.html#builder
> > >     - UBootBuilder class: https://tbot.tools/modules/tc.html#tbot.tc.uboot.UBootBuilder
> > >
> > > Hope this helps!
> >
> > Yes it helps a lot, thank you. I now have it building U-Boot:
> >
> >    tbot -l kea.py -b pcduino3.py -p clean=False uboot_build interactive_uboot
> >
> > I sent a tiny patch to tbot to fix an error I had, and I made a few
> > minor changes to what you sent.
> >
> > https://github.com/sglass68/ubtest/commit/7da7a3794d505e970e0e21a9b6ed3a7e5f2f2190
>
> Huh, actually I messed up in the patch I sent you.  Please apply the following
> diff:
>
> diff --git a/kea.py b/kea.py
> index a3ca968dc41e..f65d91b67f1d 100644
> --- a/kea.py
> +++ b/kea.py
> @@ -39,7 +39,7 @@ class KeaLab(
>          # toolchain is identified by a (unique) string.  For pcduino3 in this
>          # example, a toolchain named `armv7-a` is defined.
>          return {
> -            "armv7-a": ArmV7Toolchain,
> +            "armv7-a": ArmV7Toolchain(),
>          }
>
>      def build(self):
>
> This will make everything work as intended and the patch you needed in tbot is
> also no longer necessary (Passing a class instead of an instance will not
> bind the method's `self` argument so you got an error from a weird place.
> Maybe I should add a check against this kind of mistake ...).

OK that fixes it, thanks.

Actually I can call 'buildman -A <board>' to get the toolchain prefix
for a board, so perhaps I should figure out how to work that in
somehow, and just have a generic toolchain problem,

>
> > So my next question is how to load U-Boot onto the board. I can see
> > methods for poweron/poweroff but not for actually writing to the
> > board. Is there a built-in structure for this? I cannot find it.
> >
> > I am hoping that the Pcduino3 class can define a method like
> > 'load_uboot()' to write U-Boot to the board?
>
> I added something similar to this in our DENX internal tbot configurations but
> did not yet publish it anywhere (maybe I should add it to tbot_contrib?).  Just
> for you, here is what I did:

To me this should be a core feature, not contrib.

I wonder if there is an upstream for tbot labs? I think it would be
useful to have a directory containing people's labs like we do for
pytest. Then we can end up with common functions for doing things.

>
> 1. In the board configs, each U-Boot class defines a `flash()` method:
>
>     from tbot.tc import shell, git
>
>     class P2020RdbUBoot(board.Connector, board.UBootShell):
>         name = "p2020rdb-uboot"
>         prompt = "=> "
>         build = P2020RdbUBootBuilder()
>
>         def flash(self, repo: git.GitRepository) -> None:
>             self.env("autoload", "no")
>             self.exec0("dhcp")
>             self.env("serverip", "192.168.1.1")
>
>             tftpdir = self.host.fsroot / "tftpboot" / "p2020rdb" / "tbot2"
>             if not tftpdir.is_dir():
>                 self.host.exec0("mkdir", "-p", tftpdir)
>
>             name = "u-boot-with-spl.bin"
>             tbot.log.message(f"Fetching {name} ...")
>             shell.copy(repo / name, tftpdir / name)
>
>             addr = 0x10000000
>             self.exec0("tftp", hex(addr), "p2020rdb/tbot2/" + name)
>             size = int(self.env("filesize"), 16)
>             tbot.log.message(f"Downoad succeeded:\n  U-Boot: {size} bytes")
>
>             tbot.log.message("Flashing ...")
>             self.exec0("nand", "device", "0")
>             self.exec0("nand", "erase.spread", "0", hex(size))
>             self.exec0("nand", "write", hex(addr), "0", hex(size))
>
>    As you can see, it get the repository (as a GitRepository [1]) where
>    U-Boot was built, passed as an argument, and copies the relevant
>    binaries to a tftp directory on the lab-host (using shell.copy() [2]).
>    Then it downloads and flashes U-Boot like you would manually.
>
>    Alternatively, I could imagine the implementation connecting to
>    a hardware debugger and uploading the binary to flash that way.

Thanks very much. That explained it for me. I tried this out and ended
up moving it to the board class instead of the U-Boot class, since I
don't know what state the board is in. I just want to turn it off and
program it.

I think a lot of my confusion is all the classes and inheritance and
things like selectable.py where objects are updated outside the
module. Plus all the decorators, getattr/setattr...

>
> 2. I defined a few testcases for flashing.  Namely, `uboot_build_and_flash`
>    (build U-Boot and then flash the new version) and `uboot_bare_flash` (don't
>    rebuild, just flash the artifacts from the last build).  These then use the
>    board-specific U-Boot builder and flash method.
>
>    I also added a feature where you can specify `-fsafe` on the tbot
>    command-line.  This will, in case something goes wrong while flashing,
>    drop you onto the interactive U-Boot command-line instead of powering
>    off the board and reporting a failure.
>
>    I uploaded the testcases here:
>
>         https://gitlab.denx.de/snippets/10

OK thanks. I am slowly getting the hang of it.

My current problem is how to execute Python code on the host (the
machine that has the hardware attached). I end up having to call
host.exec() but it if I want to open a file and modify it, I cannot do
that easily remotely. Is it possible to run a tbot function on the lab
machine?

Regards,
Simon
Harald Seiler March 23, 2020, 10:30 a.m. UTC | #3
Hello Simon,

On Sun, 2020-03-22 at 12:42 -0600, Simon Glass wrote:
> Hi Harald,
> 
> On Sun, 22 Mar 2020 at 03:56, Harald Seiler <hws at denx.de> wrote:
> > Hi Simon,
> > 
> > On Sat, 2020-03-21 at 13:07 -0600, Simon Glass wrote:
[...]
> > 
> > Huh, actually I messed up in the patch I sent you.  Please apply the following
> > diff:
> > 
> > diff --git a/kea.py b/kea.py
> > index a3ca968dc41e..f65d91b67f1d 100644
> > --- a/kea.py
> > +++ b/kea.py
> > @@ -39,7 +39,7 @@ class KeaLab(
> >          # toolchain is identified by a (unique) string.  For pcduino3 in this
> >          # example, a toolchain named `armv7-a` is defined.
> >          return {
> > -            "armv7-a": ArmV7Toolchain,
> > +            "armv7-a": ArmV7Toolchain(),
> >          }
> > 
> >      def build(self):
> > 
> > This will make everything work as intended and the patch you needed in tbot is
> > also no longer necessary (Passing a class instead of an instance will not
> > bind the method's `self` argument so you got an error from a weird place.
> > Maybe I should add a check against this kind of mistake ...).
> 
> OK that fixes it, thanks.
> 
> Actually I can call 'buildman -A <board>' to get the toolchain prefix
> for a board, so perhaps I should figure out how to work that in
> somehow, and just have a generic toolchain problem,

That's not too difficult, actually.  I suggest creating a generic UBootBuilder
class from which your individual board's builders inherit:

    import abc
    import contextlib

    class BuildmanUBootBuilder(uboot.UBootBuilder):
        @abc.abstractmethod
        @property
        def buildman_board(self) -> str:
            raise Exception("abstract method")

        # Overload `do_toolchain` to customize toolchain behavior
        @contextlib.contextmanager
        def do_toolchain(self, bh):
            prefix = bh.exec0("buildman", "-A", self.buildman_board)

            with bh.subshell():
                # Setup toolchain here
                bh.env("ARCH", "arm")

                yield None

    # and for each board

    class MyBoardUBootBuilder(BuildmanUBootBuilder):
        buildman_board = "tegra"

I have never used buildman myself so I am not really sure about the
details of what needs to be done here ... But if this would be a useful
feature I could look into supporting it directly in the UBootBuilder
class.

> > > So my next question is how to load U-Boot onto the board. I can see
> > > methods for poweron/poweroff but not for actually writing to the
> > > board. Is there a built-in structure for this? I cannot find it.
> > > 
> > > I am hoping that the Pcduino3 class can define a method like
> > > 'load_uboot()' to write U-Boot to the board?
> > 
> > I added something similar to this in our DENX internal tbot configurations but
> > did not yet publish it anywhere (maybe I should add it to tbot_contrib?).  Just
> > for you, here is what I did:
> 
> To me this should be a core feature, not contrib.

I guess the split of what belongs where is not yet clearly defined and I still
need to work on separating the different components better.  For me, core is
just the code for interacting with machines; anything that looks like
a testcase should be separate from that.  Right now I've got tbot_contrib for
this but I guess the naming might not be optimal ...

By the way, are you subscribed to tbot at lists.denx.de?  I'll be posting updates
regarding this (and other things) over there.

> I wonder if there is an upstream for tbot labs? I think it would be
> useful to have a directory containing people's labs like we do for
> pytest. Then we can end up with common functions for doing things.

Maybe it makes sense to add a place in upstream tbot for these as well.
Thanks for the idea!

> > 1. In the board configs, each U-Boot class defines a `flash()` method:
> > 
> >     from tbot.tc import shell, git
> > 
> >     class P2020RdbUBoot(board.Connector, board.UBootShell):
> >         name = "p2020rdb-uboot"
> >         prompt = "=> "
> >         build = P2020RdbUBootBuilder()
> > 
> >         def flash(self, repo: git.GitRepository) -> None:
> >             self.env("autoload", "no")
> >             self.exec0("dhcp")
> >             self.env("serverip", "192.168.1.1")
> > 
> >             tftpdir = self.host.fsroot / "tftpboot" / "p2020rdb" / "tbot2"
> >             if not tftpdir.is_dir():
> >                 self.host.exec0("mkdir", "-p", tftpdir)
> > 
> >             name = "u-boot-with-spl.bin"
> >             tbot.log.message(f"Fetching {name} ...")
> >             shell.copy(repo / name, tftpdir / name)
> > 
> >             addr = 0x10000000
> >             self.exec0("tftp", hex(addr), "p2020rdb/tbot2/" + name)
> >             size = int(self.env("filesize"), 16)
> >             tbot.log.message(f"Downoad succeeded:\n  U-Boot: {size} bytes")
> > 
> >             tbot.log.message("Flashing ...")
> >             self.exec0("nand", "device", "0")
> >             self.exec0("nand", "erase.spread", "0", hex(size))
> >             self.exec0("nand", "write", hex(addr), "0", hex(size))
> > 
> >    As you can see, it get the repository (as a GitRepository [1]) where
> >    U-Boot was built, passed as an argument, and copies the relevant
> >    binaries to a tftp directory on the lab-host (using shell.copy() [2]).
> >    Then it downloads and flashes U-Boot like you would manually.
> > 
> >    Alternatively, I could imagine the implementation connecting to
> >    a hardware debugger and uploading the binary to flash that way.
> 
> Thanks very much. That explained it for me. I tried this out and ended
> up moving it to the board class instead of the U-Boot class, since I
> don't know what state the board is in. I just want to turn it off and
> program it.

During `flash()` (in my implementation), the board is powered on and waiting
for commands at the U-Boot prompt.  I know this won't work for the case where
U-Boot is failing to boot.  Alternatively, I could change it so the
implementation decides on its own whether it needs U-Boot running.  E.g.:

    class MyBoardUBoot(board.Connector, board.UBootShell):
        @classmethod
        def flash(cls, board: board.Board, repo: git.GitRepository):
            # Board is powered on.  An implementation using a debugger could
            # now flash U-Boot.

            with board.host.run("arm-linux-gnu-gdb", elf) as gdb:
                gdb.read_until_prompt("(gdb) ")

                gdb.sendline("target remote localhost:3333")
                gdb.read_until_prompt("(gdb) ")

                gdb.sendline("load")
                gdb.read_until_prompt("(gdb) ")

                gdb.sendline("quit")
                gdb.terminate0()

            # An implementation like mine above initializes the UBoot machine
            # and then continues like before:

            with cls(board) as ub:
                ub.exec0("dhcp")
                ub.exec0("nand", "update", ...)

Hope this makes sense ...

> I think a lot of my confusion is all the classes and inheritance and
> things like selectable.py where objects are updated outside the
> module. Plus all the decorators, getattr/setattr...

Ideally I'd want all that to stay hidden from the user.  I know that right
now I have not really archieved that and the code is really messy in some
places because of it.

I'd love some feedback on the things that aren't explained well in the
documentation or places where features are lacking and you needed to dig
into the internals.

> > 2. I defined a few testcases for flashing.  Namely, `uboot_build_and_flash`
> >    (build U-Boot and then flash the new version) and `uboot_bare_flash` (don't
> >    rebuild, just flash the artifacts from the last build).  These then use the
> >    board-specific U-Boot builder and flash method.
> > 
> >    I also added a feature where you can specify `-fsafe` on the tbot
> >    command-line.  This will, in case something goes wrong while flashing,
> >    drop you onto the interactive U-Boot command-line instead of powering
> >    off the board and reporting a failure.
> > 
> >    I uploaded the testcases here:
> > 
> >         https://gitlab.denx.de/snippets/10
> 
> OK thanks. I am slowly getting the hang of it.
> 
> My current problem is how to execute Python code on the host (the
> machine that has the hardware attached). I end up having to call
> host.exec() but it if I want to open a file and modify it, I cannot do
> that easily remotely. Is it possible to run a tbot function on the lab
> machine?

The idea is that you shouldn't ever need to run Python code on the
lab-host (so we don't have to make assumptions about the environment of
the lab-host). What things do you need to do that can't be done right now?
I'd argue that tbot should provide API to do them without manual (Python)
scripting on the lab-host.

For accessing files in particular, I just added some methods
(`read_text()` [1], `write_text()` [2], `read_bytes()` [3], and
`write_bytes` [4]) a few days ago.  Maybe they can help?

[1]: http://tbot.tools/modules/machine_linux.html#tbot.machine.linux.Path.read_text
[2]: http://tbot.tools/modules/machine_linux.html#tbot.machine.linux.Path.write_text
[3]: http://tbot.tools/modules/machine_linux.html#tbot.machine.linux.Path.read_bytes
[4]: http://tbot.tools/modules/machine_linux.html#tbot.machine.linux.Path.write_bytes
Simon Glass March 26, 2020, 4:19 p.m. UTC | #4
Hi Harald,

On Mon, 23 Mar 2020 at 04:30, Harald Seiler <hws at denx.de> wrote:
>
> Hello Simon,
>
> On Sun, 2020-03-22 at 12:42 -0600, Simon Glass wrote:
> > Hi Harald,
> >
> > On Sun, 22 Mar 2020 at 03:56, Harald Seiler <hws at denx.de> wrote:
> > > Hi Simon,
> > >
> > > On Sat, 2020-03-21 at 13:07 -0600, Simon Glass wrote:
> [...]
> > >
> > > Huh, actually I messed up in the patch I sent you.  Please apply the following
> > > diff:
> > >
> > > diff --git a/kea.py b/kea.py
> > > index a3ca968dc41e..f65d91b67f1d 100644
> > > --- a/kea.py
> > > +++ b/kea.py
> > > @@ -39,7 +39,7 @@ class KeaLab(
> > >          # toolchain is identified by a (unique) string.  For pcduino3 in this
> > >          # example, a toolchain named `armv7-a` is defined.
> > >          return {
> > > -            "armv7-a": ArmV7Toolchain,
> > > +            "armv7-a": ArmV7Toolchain(),
> > >          }
> > >
> > >      def build(self):
> > >
> > > This will make everything work as intended and the patch you needed in tbot is
> > > also no longer necessary (Passing a class instead of an instance will not
> > > bind the method's `self` argument so you got an error from a weird place.
> > > Maybe I should add a check against this kind of mistake ...).
> >
> > OK that fixes it, thanks.
> >
> > Actually I can call 'buildman -A <board>' to get the toolchain prefix
> > for a board, so perhaps I should figure out how to work that in
> > somehow, and just have a generic toolchain problem,
>
> That's not too difficult, actually.  I suggest creating a generic UBootBuilder
> class from which your individual board's builders inherit:
>
>     import abc
>     import contextlib
>
>     class BuildmanUBootBuilder(uboot.UBootBuilder):
>         @abc.abstractmethod
>         @property
>         def buildman_board(self) -> str:
>             raise Exception("abstract method")
>
>         # Overload `do_toolchain` to customize toolchain behavior
>         @contextlib.contextmanager
>         def do_toolchain(self, bh):
>             prefix = bh.exec0("buildman", "-A", self.buildman_board)
>
>             with bh.subshell():
>                 # Setup toolchain here
>                 bh.env("ARCH", "arm")
>
>                 yield None
>
>     # and for each board
>
>     class MyBoardUBootBuilder(BuildmanUBootBuilder):
>         buildman_board = "tegra"
>
> I have never used buildman myself so I am not really sure about the
> details of what needs to be done here ... But if this would be a useful
> feature I could look into supporting it directly in the UBootBuilder
> class.
>
> > > > So my next question is how to load U-Boot onto the board. I can see
> > > > methods for poweron/poweroff but not for actually writing to the
> > > > board. Is there a built-in structure for this? I cannot find it.
> > > >
> > > > I am hoping that the Pcduino3 class can define a method like
> > > > 'load_uboot()' to write U-Boot to the board?
> > >
> > > I added something similar to this in our DENX internal tbot configurations but
> > > did not yet publish it anywhere (maybe I should add it to tbot_contrib?).  Just
> > > for you, here is what I did:
> >
> > To me this should be a core feature, not contrib.
>
> I guess the split of what belongs where is not yet clearly defined and I still
> need to work on separating the different components better.  For me, core is
> just the code for interacting with machines; anything that looks like
> a testcase should be separate from that.  Right now I've got tbot_contrib for
> this but I guess the naming might not be optimal ...
>
> By the way, are you subscribed to tbot at lists.denx.de?  I'll be posting updates
> regarding this (and other things) over there.
>
> > I wonder if there is an upstream for tbot labs? I think it would be
> > useful to have a directory containing people's labs like we do for
> > pytest. Then we can end up with common functions for doing things.
>
> Maybe it makes sense to add a place in upstream tbot for these as well.
> Thanks for the idea!
>
> > > 1. In the board configs, each U-Boot class defines a `flash()` method:
> > >
> > >     from tbot.tc import shell, git
> > >
> > >     class P2020RdbUBoot(board.Connector, board.UBootShell):
> > >         name = "p2020rdb-uboot"
> > >         prompt = "=> "
> > >         build = P2020RdbUBootBuilder()
> > >
> > >         def flash(self, repo: git.GitRepository) -> None:
> > >             self.env("autoload", "no")
> > >             self.exec0("dhcp")
> > >             self.env("serverip", "192.168.1.1")
> > >
> > >             tftpdir = self.host.fsroot / "tftpboot" / "p2020rdb" / "tbot2"
> > >             if not tftpdir.is_dir():
> > >                 self.host.exec0("mkdir", "-p", tftpdir)
> > >
> > >             name = "u-boot-with-spl.bin"
> > >             tbot.log.message(f"Fetching {name} ...")
> > >             shell.copy(repo / name, tftpdir / name)
> > >
> > >             addr = 0x10000000
> > >             self.exec0("tftp", hex(addr), "p2020rdb/tbot2/" + name)
> > >             size = int(self.env("filesize"), 16)
> > >             tbot.log.message(f"Downoad succeeded:\n  U-Boot: {size} bytes")
> > >
> > >             tbot.log.message("Flashing ...")
> > >             self.exec0("nand", "device", "0")
> > >             self.exec0("nand", "erase.spread", "0", hex(size))
> > >             self.exec0("nand", "write", hex(addr), "0", hex(size))
> > >
> > >    As you can see, it get the repository (as a GitRepository [1]) where
> > >    U-Boot was built, passed as an argument, and copies the relevant
> > >    binaries to a tftp directory on the lab-host (using shell.copy() [2]).
> > >    Then it downloads and flashes U-Boot like you would manually.
> > >
> > >    Alternatively, I could imagine the implementation connecting to
> > >    a hardware debugger and uploading the binary to flash that way.
> >
> > Thanks very much. That explained it for me. I tried this out and ended
> > up moving it to the board class instead of the U-Boot class, since I
> > don't know what state the board is in. I just want to turn it off and
> > program it.
>
> During `flash()` (in my implementation), the board is powered on and waiting
> for commands at the U-Boot prompt.  I know this won't work for the case where
> U-Boot is failing to boot.  Alternatively, I could change it so the
> implementation decides on its own whether it needs U-Boot running.  E.g.:
>
>     class MyBoardUBoot(board.Connector, board.UBootShell):
>         @classmethod
>         def flash(cls, board: board.Board, repo: git.GitRepository):
>             # Board is powered on.  An implementation using a debugger could
>             # now flash U-Boot.
>
>             with board.host.run("arm-linux-gnu-gdb", elf) as gdb:
>                 gdb.read_until_prompt("(gdb) ")
>
>                 gdb.sendline("target remote localhost:3333")
>                 gdb.read_until_prompt("(gdb) ")
>
>                 gdb.sendline("load")
>                 gdb.read_until_prompt("(gdb) ")
>
>                 gdb.sendline("quit")
>                 gdb.terminate0()
>
>             # An implementation like mine above initializes the UBoot machine
>             # and then continues like before:
>
>             with cls(board) as ub:
>                 ub.exec0("dhcp")
>                 ub.exec0("nand", "update", ...)
>
> Hope this makes sense ...
>
> > I think a lot of my confusion is all the classes and inheritance and
> > things like selectable.py where objects are updated outside the
> > module. Plus all the decorators, getattr/setattr...
>
> Ideally I'd want all that to stay hidden from the user.  I know that right
> now I have not really archieved that and the code is really messy in some
> places because of it.
>
> I'd love some feedback on the things that aren't explained well in the
> documentation or places where features are lacking and you needed to dig
> into the internals.
>
> > > 2. I defined a few testcases for flashing.  Namely, `uboot_build_and_flash`
> > >    (build U-Boot and then flash the new version) and `uboot_bare_flash` (don't
> > >    rebuild, just flash the artifacts from the last build).  These then use the
> > >    board-specific U-Boot builder and flash method.
> > >
> > >    I also added a feature where you can specify `-fsafe` on the tbot
> > >    command-line.  This will, in case something goes wrong while flashing,
> > >    drop you onto the interactive U-Boot command-line instead of powering
> > >    off the board and reporting a failure.
> > >
> > >    I uploaded the testcases here:
> > >
> > >         https://gitlab.denx.de/snippets/10
> >
> > OK thanks. I am slowly getting the hang of it.
> >
> > My current problem is how to execute Python code on the host (the
> > machine that has the hardware attached). I end up having to call
> > host.exec() but it if I want to open a file and modify it, I cannot do
> > that easily remotely. Is it possible to run a tbot function on the lab
> > machine?
>
> The idea is that you shouldn't ever need to run Python code on the
> lab-host (so we don't have to make assumptions about the environment of
> the lab-host). What things do you need to do that can't be done right now?
> I'd argue that tbot should provide API to do them without manual (Python)
> scripting on the lab-host.
>
> For accessing files in particular, I just added some methods
> (`read_text()` [1], `write_text()` [2], `read_bytes()` [3], and
> `write_bytes` [4]) a few days ago.  Maybe they can help?

Thanks for all of this.

I will follow up on the tbot mailing list once I have figure out the
toolchain stuff.

Regards,
Simon
diff mbox series

Patch

diff --git a/kea.py b/kea.py
index a3ca968dc41e..f65d91b67f1d 100644
--- a/kea.py
+++ b/kea.py
@@ -39,7 +39,7 @@  class KeaLab(
         # toolchain is identified by a (unique) string.  For pcduino3 in this
         # example, a toolchain named `armv7-a` is defined.
         return {
-            "armv7-a": ArmV7Toolchain,
+            "armv7-a": ArmV7Toolchain(),
         }
 
     def build(self):