search

Nitrokey / pynitrokey

Python client for Nitrokey devices
Apache License 2.0
94 stars 28 forks source link

Spsdk vendor #519

Open sosthene-nitrokey opened 3 months ago

sosthene-nitrokey commented 3 months ago

Vendor parts of SPSDK needed for LPC55 updates.

Similar to https://github.com/Nitrokey/pynitrokey/pull/346

This is currently in a state that works (validating and uploading updates tested), but I can likely remove a lot of code and still keep it working. Ideally we don't need such a large blob.

spsdk is licensed BSD-3 clause, so it's compatible.

The goal is to fix #512

robin-nitrokey commented 3 months ago

Maintaining the fork in a separate repository that is forked from spsdk would make it easier to pull in upstream changes. It could still be included as a submodule here so that we don’t need to release it separately. What do you think?

sosthene-nitrokey commented 3 months ago

Would making it a separate project not bring packaging issues?

daringer commented 3 months ago

let's pull it in first, like nrf_upload - once we pull out the cli we should think how and if this should also be separate...

sosthene-nitrokey commented 3 months ago

I've been able to minimize it even more. This allowed removing the gmssl dependency. We are also rid of pypemicro.

More pruning could be done by removing Class methods (currently no class or file can be removed without breaking stuff).

The imported modules are not type checked because the type definitions are broken (223 errors).

robin-nitrokey commented 3 months ago

Regading mypy, are there still errors when disabling strict checks?

https://github.com/Nitrokey/pynitrokey/blob/a76b1b17e099bb65b6e321d97089275a2d47fea5/pyproject.toml#L83-L98

sosthene-nitrokey commented 3 months ago

There are still a couple of errors, but much less (36).

Maybe I can try to fix them.

sosthene-nitrokey commented 3 months ago

I've fixed the mypy errors in spsdk, bumped mypy to 1.9 and cryptography to 42.

daringer commented 3 months ago

libusbsio is another weird nxp package, which essentially just wraps hid, right ? would it make sense to take a look on removing that?

sosthene-nitrokey commented 3 months ago

libusbsio wraps a custom binary library by NXP. Ripping it off would be a bit more involved.

This does mean it would be great to get rid of it though.

sosthene-nitrokey commented 3 months ago

We could indeed get it to work with just hidapi, the wrapper appears very thin.

robin-nitrokey commented 3 months ago

The tricky part with replacing libusbsio is testing on Windows and Mac. I remember that we initially had some problems with the bootloader communication on other platforms than Linux so we should pay close attention not to accidentally break it with refactoring.

sosthene-nitrokey commented 3 months ago

I'm not sure I can test on mac. Maybe we should merge the version still using libusbsio at first.

robin-nitrokey commented 3 months ago

Yes, I would prefer that.