machine: u-boot: Add a hacky workaround for the crc32 command

[Rahix]

The crc32 command in U-Boot unfortunately prints the character sequence => as part of its output. This character sequence is an often used U-Boot prompt, which means tbot's prompt-searching code gets confused by crc32 output.

Unfortunately, there is no real solution to this. At least none that works universally. So to at least ease the pain a bit for people running into this, attempt automatically applying a workaround for this specific situation.

Closes #111.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.40%. Comparing base (7926f7d) to head (4bb67cf).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
+ Coverage   62.27%   62.40%   +0.12%     
==========================================
  Files          53       53              
  Lines        3746     3756      +10     
==========================================
+ Hits         2333     2344      +11     
+ Misses       1413     1412       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Rahix]

Hey @Adnan-Elhammoudi, I've been thinking about #111 for a bit. I came up with this hacky "solution" to make things a bit easier. Could you give this a test, to see whether it works as well as the workaround suggested in the issue? I.e. drop the workaround you added and rerun with the bare crc32 command. tbot should print a warning about it, but make it work correctly on its own.

[Adnan-Elhammoudi]

Hey @Rahix, it works as a charm, with the bare crc32 command:

        fileaddr =  ub.env("fileaddr")
        filesize =  ub.env("filesize")
        output = ub.exec0("crc32", f"{fileaddr}" ,f"{filesize}")
        assert expected_crc in output

but it does not drop any warning!

[Rahix]

Hm, that's unexpected! Are you really sure there is no Applying workaround for `crc32` command in U-Boot! message anywhere? If not, what is your prompt string for U-Boot? Maybe you have changed it to something different than prompt = "=> "?

[Adnan-Elhammoudi]

oh, I was on another board my bad!
I can see the message now!

│   ├─[device-u-boot] printenv fileaddr
│   │    ## 
│   │    ## fileaddr=80280000
│   ├─[device-u-boot] printenv filesize
│   │    ## 
│   │    ## filesize=2c
│   ├─Warning: Applying workaround for `crc32` command in U-Boot!
│   │   See https://github.com/rahix/tbot/issues/111 for details.
│   ├─[device-u-boot] crc32 '=80280000␍' '=2c␍'
│   │    ## 
│   │    ## > '
│   │    ## crc32 for 00000000 ... ffffffffffffffff ==> 00000000
FAILEDPOWEROFF (imx8q)

but if you notice the printenv command doesn't get the value correctly!
also it does not reproduce the same behavior on another board
the only difference is the U-boot prompt this board has "=>" the other one with "u-boot=>"

[Rahix]

Hm, that's not looking right... :(

the only difference is the U-boot prompt this board has "=>" the other one with "u-boot=>"

If your U-Boot prompt is u-boot=> , then we do not have any problem at all - this prompt won't interfere with crc32, so no warning and no workaround is needed. The problem only shows up when using => as the prompt.

but if you notice the printenv command doesn't get the value correctly!

This looks very wrong... I assume this didn't happen before the fix? Maybe, just to be sure, you can run once again, but from tbot commit 910eeb7? That version should fail at crc32 again but I'd hope the printenv still works there...

In any case, it seems

tbot/tbot/machine/board/uboot.py

Line 351 in 910eeb7

return output[len(var) + 1 : -1]

does not get the output it expects and thus cuts it apart in the wrong location... There seems to be an additional leading character, which pushes the slicing to the left such that it includes the = in the beginning. Then, there must be an unexpected trailing character which causes the inclusion of a \r at the end of the resulting string. I'll try to reproduce...

[Rahix]

So while trying to reproduce your behavior (so far I wasn't able to), I did find another inconsistency in this patch. I've fixed it now, maybe retry with the latest version of this branch to see whether this has an effect on your problem?

[Adnan-Elhammoudi]

This looks very wrong... I assume this didn't happen before the fix? Maybe, just to be sure, you can run once again, but from tbot commit 910eeb7? That version should fail at crc32 again but I'd hope the printenv still works there...

it reproduces the same behavior with this commit [910eeb7(https://github.com/Rahix/tbot/commit/910eeb7a323c45da01c5b7309340d6a6f943c087)

I assume this didn't happen before the fix?

I was using this snippet as a workaround:

        pattern = re.compile(r'=(\w+)')
        fileaddr =  pattern.findall(ub.env("fileaddr"))
        filesize =  pattern.findall(ub.env("filesize"))
        
        with ub.ch.with_prompt('\n=> '):
            output = ub.exec0("crc32", f"{fileaddr[0]}" ,f"{filesize[0]}")
        assert expected_crc in output

maybe retry with the latest version of this branch to see whether this has an effect on your problem?

no effect with this branch either tbot 0.10.7.dev13+gfe78bfa!

[Rahix]

Huh, okay... This is certainly not the intended behavior. Can you open an issue about the broken ub.env() please? And while you're at it, please rerun the testcase with an additional -v to log channel i/o. I'll take a look then.

[Adnan-Elhammoudi]

opened a new issue ticket #113 (comment)

[Rahix]

Going to merge this now under the assumption that it is working well enough. If regressions are caused by this, please speak up!