pkg: micropython: initial commit

[kaspar030]

This PR adds Micro Python as pkg (and a tests/ example).

It works on native, and I tried arduino due and iot-lab_M3. It should work on all 32bit patforms with enough (~12k, ~95k) RAM and flash.

The port doesn't do much as no libraries are included in the image.

• the garbage collector doesn't consider register contents, so it will probably not work correctly
  - [ ] there needs to be either a mapping for riot libs -> python libs or an adaption of micropython's "pyb." lib
  - [ ] integrate with VFS (postponed)

- compile micropython for 16bit platforms depending on RIOT platform (probably just needs too much memory)

UPDATE 07/2019:

• rebased to master
• rebased to current MicroPython master
• basic timer support using xtimer has been implemented
• machine.Pin() should work, if passed the right (integer) parameter as pin number
• added a basic test script

Needs #11841 for its git describe-fix. (#11841 is in)

[OlegHahm]

awesome

[kaspar030]

I PR'ed this upstream: micropython/micropython#1249

[kaspar030]

the "help()" problem somehow only shows with pyterm. Using picocom everything works fine.

[OlegHahm]

pyterm handles help particularly because otherwise it would be handled by Python's cmd module. That may cause the problem.

[kaspar030]

@OlegHahm Lol! You know the feeling, staring at the debugger, debugging something that gets affected by debugging, but it should work, thinking, "WTF'ing hell is happening here!?" ;)

[OlegHahm]

Sorry for that. I should looked at the todos...

[phiros]

Needs rebase

[miri64]

I'm still really excited for this! Let's try to get it in for 2016.07!

[miri64]

ping?

[aabadie]

Any chance to see this in the next release ? (+1 on my side)

[OlegHahm]

I heard somewhere that you could do cool stuff with (Micro)Python on RIOT...

[miri64]

@aabadie @kaspar030 so how do we resolve here?

move the pkg back to kaspar030/micropython.

and from #2968 (comment):

Could we have this pulled directly from upstream with our patches/contrib added on top?

Nope, that is a maintanance nightmare.

May I ask how a orderly set of patches is a bigger maintanence nightmare than pointing to some "random guy's" fork? You still have to rebase the patches (aka commits) every time you update the package. Granted, these are 3 commands (2 if you know what your doing and don't use the rm) more per update

git rebase -i upstream/master

vs.

git rebase -i upstream/master
rm *.patch
git format-patch upstream/master
cp *.patch ../RIOT/pkg/micropython/patches

but in the end it is way better understandable and transparent what modifications were done to the upstream right from the RIOT repository. Yes, there were reviewers being confused about patches in the past, because GitHub renders the change confusingly, but that is the only "nightmare" I can think of. This however, is more about personal habits than maintainability.

Another thing to consider is maintenance by other users: If I update the pkg with your approach I either have to fork it myself and then bend it over to my repo (giving the users no transparency about my changes) or I have to PR against your fork, making you the bottleneck for the new update still. With patches, any maintainer of RIOT-OS can review and merge them and everyone knows with one look (or more looks if the patch is bigger) what I did.

[kaspar030]

May I ask how a orderly set of patches is a bigger maintanence nightmare than pointing to some "random guy's" fork

Well, because the workflow for doing changes with patches vs fork changes from:

1. check out fork
2. make change
3. pr/push
4. (reviewer) review in github, test by locally changing pkg commit
5. in case of change request, push fixups to pr
6. pr pkg commit change

to

1. checkout upstream
2. reset to correct base revision
3. apply patches
4. do change
5. create patch
6. pr patch to RIOT
7. (reviewer) repeat 1-3 for reviewing. comments only possible on patch.
8. in case of change request, push change commit to pkg branch, create new patch, push new patch to RIOT PR, reviewer has to manually apply additional patch, ...
9. when done, squash pkg branch, ...

Essentially the whole github workflow is broken when using patches.

[kaspar030]

I disagree with this. The RIOT_TERMINAL is only required for the case where the micropython REPL shell is used

this

[kaspar030]

• rebased, squashed

[aabadie]

May I ask how a orderly set of patches is a bigger maintanence nightmare than pointing to some "random guy's" fork

In the case of this PR, no patches would be required since the RIOT port is only adding adaption files to micropython upstream (in ports/riot). I have a branch ;) that adds the adaption code in a subfolder of the RIOT package and copy the these files on the fly in the upstream micropython ports/riot directory. That could work indeed and it's self contained.

This PR is based on a branch in @kaspar030's fork that is used in a PR to micropython upstream (micropython/micropython#1249). Once the micropython PR is merged, we can just update the package URL/VERSION in RIOT to point to the upstream micropython url /commit.
In this case, if you want to extend the micropython port of RIOT, just PR directly to micropython, then update the RIOT package version.

[aabadie]

https://github.com/RIOT-OS/RIOT/pull/2968/files#r347873773 is still unaddressed.

[miri64]

Well, because the workflow for doing changes with patches vs fork changes from:

You could still have a fork (I used to do this with lwIP before lwIP got a better porting layer / our make system got a better handling for pkg e.g.) and use patches... Also idea is to keep the set of patches minimal of course. All major changes must either be provided to upstream or be provided in the adaption layer of the package. In my experience if you can't provide to upstream, can't do an adaption layer or have a huge, unmaintainable set of patches, you are doing something wrong.

Why does any other operating system find patches just fine to use with packages, while for RIOT according to you it is a maintenance burden?

[haukepetersen]

Why does any other operating system find patches just fine to use with packages, while for RIOT according to you it is a maintenance burden?

No idea, but IMHO patches are hell.

[kaspar030]

You could still have a fork and use patches.

Yeah, to me, having the extra layer of pulling the source from github (or optionally a local folder) is much less work than dealing with patches, especially when collaborating. patches are always on top of what git/github already provides. Reviewing changes to patches is extremely cumbersome.

[aabadie]

In case you missed #2968 (comment), there's no need for patches in the micropython port. The contributed files are orthogonal with the current micropython code base: they are put in a separate subdirectory.

But I find it nice to contribute the RIOT adaption layer upstream: it makes it visible to micropython that a port for RIOT exists.

[aabadie]

This section should be removed now: it's duplicated with the new one added below.

[kaspar030]

yup. I removed the old one and moved the updated one up.

[miri64]

You could still have a fork and use patches.

Yeah, to me, having the extra layer of pulling the source from github (or optionally a local folder) is much less work than dealing with patches, especially when collaborating. patches are always on top of what git/github already provides. Reviewing changes to patches is extremely cumbersome.

To me it is a matter of consistency and traceability. Let's say we have contributor evilhaxxor provide a package for a security relevant package. Which would you trust more? A package pulling upstream code for a dedicated version and a small set of well placed patches or a package that pulls in some commit from a fork of evilhaxxor? Yes you can go to the fork of evilhaxxor and look for the commit it is pointing to and the dig through the history to understand the changes, but that seems way more complicated then having the changes just there and reviewed in RIOT.

I also don't get the PR argument. Your workflow would mean two PRs: One against the contributors fork and one against RIOT. Even worse, the PR against RIOT gives no immediate transparency about what the change is about. With patches just one PR is needed, so way lower maintenence burden for everyone involved + the patches and a well documented version jump give a better transparency about what is changed.

But granted, I'm talking more about a stable package than a package currently under development like this one or nimble (/cc @haukepetersen), so maybe this discussion is a bit misplaced, but I have a problem with the generalized statement "patches are a nightmare/hell".

@aabadie, I read your comment and think it might be a good compromise until the RIOT code is merged upstream.

[kaspar030]

But granted, I'm talking more about a stable package than a package currently under development like this one or nimble (/cc @haukepetersen), so maybe this discussion is a bit misplaced, but I have a problem with the generalized statement "patches are a nightmare/hell".

ok, then let me take back that general statement. In this case, we do expect quite some development work (until the changes are upstreamed), and the patches are quite substantial (>1300 lines spread over >20 commits), so I think in this case, we're better off with a (temporary) fork (and working with the patches would be a nightmare 😉).

[miri64]

ok, then let me take back that general statement. In this case, we do expect quite some development work (until the changes are upstreamed), and the patches are quite substantial (>1300 lines spread over >20 commits), so I think in this case, we're better off with a (temporary) fork (and working with the patches would be a nightmare wink).

Accepted. @aabadie's solution could also be a solution, but that would mean, once the changes are upstreamed they need to be removed again. Don't know what is worse 😅.

[aabadie]

Ideally the first step would be to revive micropython/micropython#1249, which just means remove WIP from the PR title and add a comment that this PR is ready. Maybe it could then be merged quite fast and the RIOT package could directly point to micropython upstream.

[aabadie]

ACK when #2968 (review) is addressed

[kaspar030]

The default RIOT_TERMINAL value could also be set there instead of the application Makefile.

I disagree with this. The RIOT_TERMINAL is only required for the case where the micropython REPL shell is used, as is the case with the example application here, but doesn't have to be the case in other situation such as automated tests.

@aabadie do you agree on this?

[kaspar030]

Ideally the first step would be to revive micropython/micropython#1249, which just means remove WIP from the PR title and add a comment that this PR is ready. Maybe it could then be merged quite fast and the RIOT package could directly point to micropython upstream.

That is parallel work. No need to wait with this PR. Also, I'd rather ask them to take another look at the upstream PR once all machine API's that @dpgeorge mentioned are actually implemented and when there's a way to compile from within MicroPython.

[aabadie]

Please squash

[bergzand]

All green, GO!

[kaspar030]

All green, GO!

Finally! 😉

Thanks everyone involved!

[OlegHahm]

Yeah! Awesome! Thanks for the great work.