feat: Add 'image erase' command

[tormodvolden]

Note I copied the # type: ignore # noqa from state_read() and state_write(), which seems to prevent a lint suggestion to split the line.

[JPHutchins]

Looks great! Does it pass mypy without the type ignore? OK if it doesn't, probably a type inference issue with that helper function.

[tormodvolden]

Without the type ignore, mypy is (still) happy:

$ mypy smpmgr/image_management.py
Success: no issues found in 1 source file

But lint wants to split the line:

-        r = await smp_request(smpclient, options, ImageErase(slot=slot), "Waiting for image erase...")
+        r = await smp_request(
+            smpclient, options, ImageErase(slot=slot), "Waiting for image erase..."
+        )

[JPHutchins]

Without the type ignore, mypy is (still) happy:

$ mypy smpmgr/image_management.py

Success: no issues found in 1 source file

But lint wants to split the line:

-        r = await smp_request(smpclient, options, ImageErase(slot=slot), "Waiting for image erase...")

+        r = await smp_request(

+            smpclient, options, ImageErase(slot=slot), "Waiting for image erase..."

+        )

That's good, in a perfect world we would not need any linter directives. Good to remove the type ignore, line split is up to you, but I like it.

[JPHutchins]

I've not used this function and I'm curious if it's 1-indexed like the other image operations? That is, slot 1 maps to "image slot 0", etc? There's an issue discussing it.

#3

[tormodvolden]

Oh, when I said "without the type ignore" above I had actually tried removing the whole "# type: ignore # noqa" including noqa. If I leave just "# noqa" lint still wants to split the line.

[JPHutchins]

Oh, when I said "without the type ignore" above I had actually tried removing the whole "# type: ignore # noqa" including noqa. If I leave just "# noqa" lint still wants to split the line.

Yeah, I suggest removing the #noqa

[tormodvolden]

But then I would need to split the line too, right? Which is fine to me, I just wanted to make it consistent with the code for the other commands.

[tormodvolden]

But why does "type: ignore" affect the line-splitting and not "noqa". Maybe I don't understand the linter override syntax.

[JPHutchins]

But why does "type: ignore" affect the line-splitting and not "noqa". Maybe I don't understand the linter override syntax.

The directive is what's causing it to overflow. Can cause a "chicken and egg" problem. I hate to ask you to do random maintenance, but the right thing would be to remove all of the unnecessary directives from the file and let black reformat it.

[tormodvolden]

Yes, the slot numbering is hard to understand. But when image read-state listed an image in slot 1, and I used erase --slot 1, it erased this image alright. I had used upload --slot 2 to upload it though.

[tormodvolden]

I'd be happy to do this random maintenance :) Do you prefer it in a separate commit for the non-erase parts?

[tormodvolden]

The directive is what's causing it to overflow

d'oh! Thanks for explaining this mystery :)

[JPHutchins]

Yes, the slot numbering is hard to understand. But when image read-state listed an image in slot 1, and I used erase --slot 1, it erased this image alright. I had used upload --slot 2 to upload it though.

To confirm, uploading to slot 2 gets listed as slot 1, and then erase slot 1 erases slot 1, which is the image slot uploaded to with argument slot 2?

There's not much to do other than document it. The idea for smpmgr is to be e reference implementation that's 1:1 with the specification, and it's the specification that caused this problem by using 0 as a default slot, causing the argument to be offset by 1 🙄. There are a few top level convenience abstractions, like "upgrade", where this could get cleaned up.

I'll think about it. Probably I need to update the upload docstring.

[tormodvolden]

To confirm, uploading to slot 2 gets listed as slot 1, and then erase slot 1 erases slot 1, which is the image slot uploaded to with argument slot 2?

Yes. But I don't know if the SMP server is not doing something funny also.

[JPHutchins]

Thank you!

[tormodvolden]

Yes. But I don't know if the SMP server is not doing something funny also.

I mean, it looks like the primary slots are not listed on this particular SMP server, so slot 1 is the secondary slot of the first DFU image (and second image because they share secondary slot). I remember from #28 that I had to specify 2 when uploading and then it listed 2 slots, but the SMP server setup may have changed since then. This ought to be tested on a SMP server reference instance and not on the hacked up SMP server that I have at hand :)

[JPHutchins]

Yes. But I don't know if the SMP server is not doing something funny also.

I mean, it looks like the primary slots are not listed on this particular SMP server, so slot 1 is the secondary slot of the first DFU image (and second image because they share secondary slot). I remember from #28 that I had to specify 2 when uploading and then it listed 2 slots, but the SMP server setup may have changed since then. This ought to be tested on a SMP server reference instance and not on the hacked up SMP server that I have at hand :)

To that end, we're very close to having some native/QEMU SMP servers available for smpclient + smpmgr CI: https://github.com/intercreate/smp-server-fixtures