Dev 5.3 Micropython bindings

[amirgon]

No description provided.

[embeddedt]

@kisvegabor Looks like a squash merge would be best here. Otherwise there are going to be a lot of commits in the history.

[amirgon]

@embeddedt squash merge loses history.
I tried to explain in every commit message what I did and why I did it. This is especially important for bug fixes (and there are quite a few there), where I explained what was the bug and how it was fixed. It's also important to see the commit as a whole including all the files affected.

Why is it important to you to "save" a few commits? Are you willing to lose valuable history for that?
It's worth noting that most of my commits are to my Micropython Binding code and not to lvgl sources, so looking at history of specific lvgl sources will not show as many commits.

[amirgon]

@kisvegabor you would need to fix the makefile on lv_mpy project if you move Micropython files.

[kisvegabor]

@amirgon Thank you very much! Ok, I'll fix makefiles we merge it.

However I can compile it:

lvgl/lv_bindings/lv_mpy/pycparser/utils/internal/constptr.c:3:10: error: assignment of read-only location ‘*arg’

Seems reasonable because this the content of the file:

void foo(char * const * arg) {
  arg += 1;
  (*arg) += 1;
}

void foo2(char ** const arg) {
  arg += 1;
  (*arg) += 1;
}

Have I missed something when moved the project?

Regarding the commit history, I agree with @amirgon. The commit messages are clear. I like to see the curve of the development.

[amirgon]

Have I missed something when moved the project?

@kisvegabor I'm not sure. I've noticed you updated pycparser version. Could this be related?
Last night before I sent the PR I tested it on my lv_mpy fork. You could try cloning it again, see if it's working for you and what's the difference between it and your fork.

[kisvegabor]

I added submodule again and it cloned the latest version of pycparser.

I examined it and found that the issue should be that the pycparser is compiled in Eclipse (I tried to build the PC simulator project with the updates). pycparser has an example and a test folder where there are some "strange" things even an other main function: https://github.com/eliben/pycparser/blob/master/examples/c_files/funky.c#L12

It seems pycparser is not intended to add to a project this way because the IDE will collect and compile all c files.

I can imagine three solutions:

1. Instead of adding pycparser as a submodule add only the required files directly. I don't like it because it's very hacky and we can't update pycparser late
2. Exclude pycparser from the sources to be compiled. But it's not beginner friendly and I'm not sure every IDE (e.g. Arduino IDE) can be configured this way.
3. Create a new a repo with lvgl and pycparser (pycparser next to lvgl not in it) which is not intended to be loaded into an IDE but to run gen_mpy.py from a terminal. On every release, we can run the scripts manually and copy an up-to-date version of the bindings to lv_bindings. The user still will have the opportunity to generate the bindins again in the mentioned repo. I vote for this solution.

[amirgon]

@kisvegabor I actually never tried to compile pycparser examples. On Micropython you have to specify explicitly the .c files you want to compile, so I don't see why someone would "accidentally" compile pycparser.
Could you explain why not keep it where it is and just avoid compiling it? All we need is to allow gen_mpy.py import it.

[kisvegabor]

If you just use lvgl (not to generate mpy bindings but to develop a GUI) the source of pycparser will be in lvgl.
Let's say you develop your GUI in Eclipse, it will automatically generate the makefile for all C files in the project (including pycparser). Which means it will try to comply pycparser too with lvgl.

[amirgon]

Let's say you develop your GUI in Eclipse, it will automatically generate the makefile for all C files in the project (including pycparser)

@kisvegabor Then we also have a problem with lv_mpy_example.c. It's not supposed to be compiled and may fail compilation if lv_conf.h is different than what it was when lv_mpy_example.c was generated.

Are you sure this is the default behavior in Eclipse - build whatever .c file it finds in whatever subdirectory?
This sounds strange. There could be multiple reasons not to compile all .c files. Some of them may only be tests or examples, some of them may not be useful for the platform you are building for etc. Building all whatever .c files you can find doesn't sounds like a very robust approach.

[kisvegabor]

@amirgon I'm sure that this is the default behavior not only in Eclipse but in the majority of the IDEs. I work this way all the time. Just create a C file, write something in it and build. You can exclude some folders somehow from the build but that is different every IDE and needs extra steps.

@embeddedt Can you confirm it?

[amirgon]

@kisvegabor Not in MSVC. There you have to explicitly add a file to a "project" in order for it to compile, which is in my opinion a more sane approach than compiling any file on the disk.

Anyway, if that is really the case, I would like to suggest another option: Move the entire lv_bindings to a new repo with lvgl sub module. We already have pycparser's .c files to exclude as well as lv_mpy_example.c and who know what's next. Whoever uses a binding would have to be smarter than building all .c files blindly. Doing that would create another hierarchy in the directory tree, but that's probably not a big deal.

[kisvegabor]

@amirgon

Not in MSVC. There you have to explicitly add a file to a "project" in order for it to compile, which is in my opinion a more sane approach than compiling any file on the disk.

In Eclipse-based IDEs, you can press F5 to refresh the files from the file system. The other solution is to add links to the folders (or maybe for files) one by one but is hidden deeply in the project properties.

Move the entire lv_bindings to a new repo with lvgl sub module.

So it would be a new repo for all the bindings? I don't expect that we will have more than 5 bindings in the long term (Mpy, Python, Lua?, Rust?) so it's still reasonable to keep them in one place.

I created the repo here: https://github.com/littlevgl/lv_bindings

[amirgon]

So it would be a new repo for all the bindings? I don't expect that we will have more than 5 bindings in the long term (Mpy, Python, Lua?, Rust?) so it's still reasonable to keep them in one place.

@kisvegabor We can start with one repo. You can always split it in the future. Having at least Python and Micropython in the same repo makes sense, not sure what other bindings are on the way. Anyone started working on some other bindings?

I created the repo here: https://github.com/littlevgl/lv_bindings

Ok. You are going to move all the commit history as well, right?

[kisvegabor]

@kisvegabor We can start with one repo. You can always split it in the future. Having at least Python and Micropython in the same repo makes sense, not sure what other bindings are on the way.

Ok, I agree.

Anyone started working on some other bindings?

I don't know, but I found the Python binding accidentally too. So who knows :)

Ok. You are going to move all the commit history as well, right?

My view is to completely remove the lv_binding folder and move its content to the new repo. In the lvgl repo it will mean only a new commit on top of these above with "move lv_bindings to a separate repo". I don't know how to "split" the history to lvgl related and mpy related parts.

[amirgon]

My view is to completely remove the lv_binding folder and move its content to the new repo. In the lvgl repo it will mean only a new commit on top of these above with "move lv_bindings to a separate repo". I don't know how to "split" the history to lvgl related and mpy related parts.

@kisvegabor I think it's important to keep the commit history, for the reasons I mentioned above as a response to @embeddedt.
A quick google search returned this, for example, for how to do it.

[kisvegabor]

@amirgon
Wow, nice! It'd be a great solution. Will you try it?

[amirgon]

@kisvegabor Ok, I'll give it a try.

[embeddedt]

@kisvegabor Yes, it's default behavior to compile everything found in the sub directory. You can exclude files though.

@amirgon When squash merging we can retain the history inside the one commit message. We would lose access to individual changes though. I think @kisvegabor should decide, it's his project.

[kisvegabor]

I vote for leaving the history.

The main reason is to show the volume of the work. For example, I often add commits like these:

• lv_canvas: fix typo
• lv_canvas: resolve warnings
• lv_canvas: minor updates

These 3 commits might change only 5 lines of code. It wouldn't be fair to have only one commit for a huge update like this.

[embeddedt]

@kisvegabor That's true. Let's do it that way then.

[amirgon]

Splitting the repos caused git history rewrite.
I'm going to try to rebase my changes.

[kisvegabor]

It seems good to me now. lvgl is compiling in Eclipse too and the history seems good as well.

[amirgon]

@kisvegabor Could you git pull my lv_bindings/dev5.3 ? I can't send you a pull request since GitHub complains that "littlevgl:master and amirgon:dev-5.3 are entirely different commit histories."

[kisvegabor]

@amirgon Done.
The history is fine too: https://github.com/littlevgl/lv_bindings/commits/master/micropython/gen_mpy.py

[amirgon]

@kisvegabor Nice.
Now you can update the lv_binding module on lv_mpy PR. lv_mpy should work now with lb_binding, I fixed the Makefile there.

[kisvegabor]

Merged!

[amirgon]

Merged!

@kisvegabor Great!
I think that at some point you would want to change the submodules to point at your fork and not at my fork. (this and this)

[kisvegabor]

I think that at some point you would want to change the submodules to point at your fork and not at my fork. (this and this)

Yes. I merged the lvgl updates to dev-5.3.
I'm planning to move lv_mpy to littlevgl and change the submodules.
Is it okay for you if I move it now?

[amirgon]

Is it okay for you if I move it now?

@kisvegabor Sure, just please make sure you move it with all the history.

[kisvegabor]

GitHub has a move feature, I hope it works well.

[kisvegabor]

Seems good: https://github.com/littlevgl/lv_mpy/commits/master