toolkit: validate arguments *after* modification is complete

[josuah]

The current toolkit modifies path on MacOS and Linux platforms. Perform this validation only after the path is modified to allow the script to be run on Linux and MacOSX without error.

Before:

$ cd alif-security-toolkit/toolkit
$ python ./app-write-mram.py
Writing MRAM with parameters:
Device Part# B1 (AB1C1F4M51820PH) - 1.8 MRAM / 2.0 SRAM - Rev: A0
- Available MRAM: 1888256 bytes
<class 'FileNotFoundError'>
Error: there is a problem with the image ../build/AppTocPackage.bin
Please check it exists and retry

After:

$ cd alif-security-toolkit/toolkit
$ python ./app-write-mram.py
Writing MRAM with parameters:
Device Part# B1 (AB1C1F4M51820PH) - 1.8 MRAM / 2.0 SRAM - Rev: A0
- Available MRAM: 1888256 bytes
[INFO] Burning: build/AppTocPackage.bin 0x8057bf50
[INFO] baud rate  56500
[INFO] dynamic baud rate change  Enabled
[INFO] /dev/ttyACM0 open Serial port success
...

P.S.: Since we I benefit from this repo I try to contribute back... but we can silently maintain our own fork to avoid burdening MicroPython with extra Alif maintenance work.

[iabdalkader]

Whoa I'm surprised to see you here!

but we can silently maintain our own fork to avoid burdening MicroPython with extra Alif maintenance work.

It's fine, improvements to the tools are welcome! How are you liking the tools so far? Are going in the right direction?

[josuah]

Whoa I'm surprised to see you here!

I like MicroPython. :) Small world!

How are you liking the tools so far? Are going in the right direction?

The ideal direction really!
The binaries would have been a lot more tedious to debug.
Adding command line flags like the previous commits will allow integrating them in a broader ecosystem.

[dpgeorge]

Thanks for the contribution!

But, would it be simpler to just adjust application_package.ds like this:

--- a/toolkit/bin/application_package.ds
+++ b/toolkit/bin/application_package.ds
@@ -1,4 +1,4 @@
-set semihosting args ../build/AppTocPackage.bin 0x8057bf50
+set semihosting args build/AppTocPackage.bin 0x8057bf50^M
 continue
 wait
 reset reset.hardware

?

[dpgeorge]

Actually, your fix is correct, the code should definitely validate the paths after modifying them.

But, IMO the modification to strip out ../ bits of the path should be removed instead. Because in some cases you may want to have ../ in the path. Paths should be correct to start with and not need any modification.

[josuah]

the modification to strip out ../ bits of the path should be removed instead

Agreed. I aimed for lightest modification to the tools assuming new releases coming, should I try again by performing a bit more rework?

[dpgeorge]

I aimed for lightest modification to the tools assuming new releases coming,

Yes, that makes a lot of sense, that's also the approach we took.

But, I think in this case, removing the code that strips out ../ is about the same size of a change as moving the validation.

should I try again by performing a bit more rework?

Yes please!

[josuah]

First an extra commit to illustrate the various locations where the ../ might be affecting.

It seems like the AlifSemi team prefers cd directory; ./script.py to directly directory/script.py (not a problem I think?), so the libraries were adapted to be callable from a subdirectory or from the base directory (this is fragile though).

The long-term solution might be to use a absolute_base_directory_path=... of some kind...

Below just the fix discussed:

[dpgeorge]

Looks good, thanks for updating!

I tested it on a board, deploying the firmware, and it still works like it did before this PR.