Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial nrf52_upload extraction from pc-nrfutil #346

Merged
merged 3 commits into from
Mar 28, 2023
Merged

Conversation

daringer
Copy link
Collaborator

@daringer daringer commented Mar 21, 2023

This PR replaces the nrfutil dependency with an extract from the originating, archived repository.

Changes

  • use nk3.bootloader.nrf52_upload instead of nrfutil to upload a signed firmware to a Nitrokey 3 Mini (nRF52)
  • avoids importing pc-ble-driver-py
  • enables python 3.11 support
  • (untested) should enable arm / *bsd support
  • adds pyserial & protobuf to dependencies
  • comes with the necessary package.py to create firmware images, although it is not used
  • applies black & isort to nrf52_upload

Checklist

  • tested with Python3.9
  • run make check or make fix for the formatting check
  • signed commits
  • updated documentation (e.g. parameter description, inline doc, docs.nitrokey)

Test Environment and Execution

  • OS: Arch
  • device's model: nk3am
  • device's firmware version: 1.3.0RC1

Fixes #330
Fixes #265

@daringer
Copy link
Collaborator Author

daringer commented Mar 22, 2023

From technical point of view I would say this is fine for an initial working state.
Did not yet test on windows... that's to be done

I added a README.md, maybe you could take a look, if this is ok from license point of view.

  • The sources are not in their original state anymore due to black and isort, don't see an issue with this
  • How about removing the license notices from each file and replacing them with spdx + the complete license within the parent directory ?
  • any other license related things to consider?

Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, I’d prefer to have this in a separate package to have a clean separation, otherwise looks good to me.

pynitrokey/nk3/bootloader/__init__.py Outdated Show resolved Hide resolved
@daringer
Copy link
Collaborator Author

windows tests still missing before we can merge ...

... separate package: Nitrokey/nrf52-uploader ?

daringer and others added 3 commits March 27, 2023 16:24
* avoids importing pc-ble-driver-py
* enables python 3.11 support
* adds `pyserial` & `protobuf` to dependencies
The pynitrokey.nk3.bootloader.nrf52_upload module is imported from
nrfutil and will be removed from this repository in an upcoming release.
Therefore we disable the flake8 and mypy checks for it.
@robin-nitrokey
Copy link
Member

Rebased onto master and added flake8 and mypy ignores to fix the CI.

@daringer daringer merged commit 6e50e5a into master Mar 28, 2023
@jans23 jans23 deleted the nrf52-uploader branch August 2, 2023 12:17
@sosthene-nitrokey sosthene-nitrokey mentioned this pull request Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python 3.11 Support pynitrokey >=0.4.26 not installable on Arm
2 participants