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

1.0.0 milestone #49

Closed
ThomasWaldmann opened this issue Aug 8, 2021 · 32 comments · Fixed by #51 or #52
Closed

1.0.0 milestone #49

ThomasWaldmann opened this issue Aug 8, 2021 · 32 comments · Fixed by #51 or #52
Milestone

Comments

@ThomasWaldmann
Copy link
Collaborator

considering the great work of @wnienhaus, this is much closer to be practically useful than previously.

so i create a milestone for first release - maybe only add very important features and bug fixes into there, so we can do a first release soon.

@wnienhaus
Copy link
Collaborator

Thank you. Thanks for the great review!

As a next step (after #43 is merged) I wanted to solve #41 - because that is a bad bug. The ULP only implements some conditions for the JUMPS instructions and the other "supported" conditions are implemented by the binutils-esp32ulp assembler, by emitting 2 instructions that emulate the desired condition, using those conditions actually supported by the ULP. I would suggest this is fixed before 1.0.

Other than that I had intended to go through the currently skipped tests in the 02_compat_rtc_tests.sh script to see if I can get them all to pass.

@ThomasWaldmann
Copy link
Collaborator Author

OK, currently all tickets in 1.0 milestone are closed, but I'll add this one for the skipped tests mentioned above.

@ThomasWaldmann ThomasWaldmann added this to the 1.0 milestone Sep 27, 2021
@wnienhaus
Copy link
Collaborator

Great. I will take on the skipped tests. Let's see how deep the rabbit hole will be.

@wnienhaus
Copy link
Collaborator

oi ... did it close due to the merge? I did not intend to have this closed yet.
There are still a number of things to fix. 3 more issues fixed already - will send PR tomorrow likely.
And potentially still a few more things.
2 test scripts left to get to pass (out of the 5 we skipped so far)

@wnienhaus
Copy link
Collaborator

Actually looking over the open issue list, I was wondering if a few small things could still be nice before version 1

Maybe these 2?

  1. document minimum required (micro)python version #48 (I tested on 1.11 and 1.16 so I think the min version is 1.11 and versions thereafter will work)
  2. The README says "There might be some stuff missing, some bugs and other symptoms of alpha software". With v1.0, maybe we can drop the reference to "alpha" and maybe we can clarify what is missing? (I only know of: assembler macro support, #including other .S files)

I was also wondering about:

  1. make imports less ugly #39
  2. add useful examples #12

For the first - any idea what the module could be called that takes over what esp32_ulp.__main__ does, and which esp32_ulp.__main__ then should call? It's not doing "assembly" and not "linking", but something that wraps those operations into a working application. esp32_ulp.app? esp32_ulp.build? Or maybe the functions should move into esp32_ulp.__init__, so that it can be imported as just esp32_ulp and other code (and also esp32_ulp.__main__) could then call a method like do_assemble or build or run with a filename?

For the second, technically some of the ulp code we're assembling with the compat test scripts are actually working examples. Should we simply document this and call that enough, or maybe just create our own simple blink example using the RTC_* methods now supported (I think I could appropriate some code I have actually running somewhere)?

But maybe these 2 last ones are just about "perfection" and they're not really that important for the v1.0 milestone. Happy to hear thoughts.

@ThomasWaldmann
Copy link
Collaborator Author

  1. I didn't run the code on an esp32 since quite a while. But I know that doing this often has revealed additional todo, because some problems only showed on the esp32. Did you run the latest code from master on esp32?

Yeah, #48 should be done before 1.0.

  1. Sure. Due to your recent work, guess we also can declare beta now.

  2. See make imports less ugly #39.

  3. Yeah, maybe move your suggestion to add useful examples #12, sounds good.

@wnienhaus
Copy link
Collaborator

Thanks for that. I have now:

  1. Re-tested everything (unit tests and compat tests) on the ESP32 directly, with MicroPython versions v1.10, v1.11, v1.12 and v1.17
  2. Updated the README regarding beta quality level
  3. Made imports less ugly as per your suggestion in make imports less ugly #39
  4. Added a blink example to blink an LED with the ULP (to cover add useful examples #12)

Should I create 1 PR per improvement above (in that case, could I ask you to merge the currently open PR #53 , so I can create PRs against that latest code), or should I simply push those "last minute changes" to the same open PR?

@ThomasWaldmann
Copy link
Collaborator Author

I've merged it. You can put stuff into a single PR, but try to keep the commits clean (as you usually do).

@ThomasWaldmann
Copy link
Collaborator Author

OK, looks like we reached 1.0! :) (reopening for remainder of todo)

What's left to do?

  • of course, tag "1.0.0" in git

  • is uploading a pip-installable release package to pypi actually helpful here or are users using a git checkout of this / zip or tgz file from github releases rather for this? I mean using pip to download and install and as a result having the code in .../lib/python3.x/... is not very helpful if the next step is uploading it to the ESP32 device.

  • advertise it on twitter / mastodon / ... (anywhere else?)

@ThomasWaldmann ThomasWaldmann changed the title 1.0 milestone 1.0.0 milestone Oct 11, 2021
@wnienhaus
Copy link
Collaborator

wnienhaus commented Oct 11, 2021

Cool!

Tagging:

  • Can you add the tag in git? I am not sure I can do that?

pip-installable:

  • I agree - not sure there's much use from a Python 3 point of view to have this pip installable. As you said, even if one would just download it with pip, it would anyway just be a bunch of files to upload to the ESP32.
  • However, MicroPython has upip which exists on the ESP32 too, and it could be a convenient way to install on the ESP32 directly on the ESP32 via the REPL.
  • I see that MicroPython modules also live in pypi and have the convention of being prefixed with micropython- (e.g. micropython-hmac), so we could call ours micropython-py-esp32-ulp?
  • Then it would be installable on the ESP32 with import upip; upip.install('micropython-py-esp32-ulp'). What do you think?
  • But... I'm not familiar with how to make something pip-installable. Are you fluent in this and want to help with this? Otherwise I'll figure it out!

Advertising:

  • Not my strong suit either. You seem to have many more twitter followers - perhaps you, also as "father of the project", could post to twitter (once all is ready) and I'll retweet (not sure my followers are a good audience, but it wouldn't hurt).
  • Perhaps Hacker News (https://news.ycombinator.com) could be another place?
  • Perhaps hackaday.io could be another place? It seems to have mostly hardware projects, but a few tutorial style projects too.
  • Is mastodon a place where I should be? :)

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Oct 11, 2021

https://github.com/fizista/micropython-umqtt.simple2 this is a recent project on pypi that supports upip.
I am familiar with normal pypi packages, but not with micropython pypi packages.

I can add the tag as soon as everything is ready.

mastodon: similar to twitter, but non-commercial, federated, FOSS. less traffic than twitter though.

HN: good idea.

@wnienhaus
Copy link
Collaborator

So. Publishing to pypi was rather easy. I needed to put a special "sdist_upip.py" into the project and use that instead of normal sdist, which takes care of gzipping with a smaller dictionary size (that upip needs). The rest was just a normal setup.py and running python3 setup.py sdist.

I then uploaded the resulting package to test.pypi.org using twine and now there's a package: https://test.pypi.org/project/micropython-py-esp32-ulp/

Testing on the ESP32 it worked and I got this:

>>> # connect wifi
>>> import network
>>> w=network.WLAN(network.STA_IF)
>>> w.active(True)
True
>>> w.connect('XXXX', 'YYYY')
>>> 
>>> import upip
>>> upip.index_urls
['https://micropython.org/pi', 'https://pypi.org/pypi']
>>> 
>>> # point upip at test pypi
>>> upip.index_urls=['https://test.pypi.org/pypi']
>>> upip.install('micropython-py-esp32-ulp')
Installing to: /lib/
Warning: test.pypi.org SSL certificate is not validated
Installing micropython-py-esp32-ulp 1.0.0 from https://test-files.pythonhosted.org/packages/6b/bd/03de3dfdc57ca5d93ad0818be422e844a413ebb0609931d2d9b236e04f4e/micropython-py-esp32-ulp-1.0.0.tar.gz
>>> 
>>> # show that files were downloaded
>>> import os
>>> os.listdir("lib/esp32_ulp")
['__init__.py', '__main__.py', 'assemble.py', 'definesdb.py', 'link.py', 'nocomment.py', 'opcodes.py', 'parse_to_db.py', 'preprocess.py', 'soc.py', 'util.py']
>>> 
>>> # show that esp32_ulp loads and from the right place
>>> import esp32_ulp
>>> esp32_ulp.__file__
'/lib/esp32_ulp/__init__.py'

The only thing I did (learned) was that I had some test .py files accidentally included in the distribution package, which I then deleted from pypi to re-upload a "cleaned" distribution package. But I then learned that filenames can never be reused on pypi. So version 1.0.0 is now no longer usable as a filename (at least on test pypi). We will need some mechanism to allow republishing a version if ever needed. For now I simply appended another number to the version, so now there's 1.0.0.1 in the index. I tried the Debian way of using 1.0.0-1 but that didnt work (or at least setuptools changed that into 1.0.0-post1, which could work I guess). Perhaps you have some best-practice suggestion how best to version distribution packages?

About the code (setup.py, et al), I will clean up and make a PR tomorrow.

PS: about advertising it: I guess https://forum.micropython.org could be another place?

@mattytrentini
Copy link

I've been watching the two of you progress py-esp32-ulp for a little while now...thank you! These contributions are great.

I've not been in a position to help but I can at least help advertise a 1.0.0 when it's ready; I run the Melbourne MicroPython Meetup, and the MicroPython Slack Channel so can make sure it's announced at both of those. There's also the Adafruit Python on Hardware newsletter which has a wide distribution and the MicroPython forum and MicroPython Discord. It might even be possible to announce it at some of the Espressif sources (email newsletter and forums) but I have less contact with those.

The only suggestion I could make before releasing 1.0.0 would perhaps be to tidy up some of the documentation - the README is good but could potentially be friendlier to newcomers.

@ThomasWaldmann
Copy link
Collaborator Author

@wnienhaus Guess we should stay with x.y.z, that is the usual thing for python packages.

As you tested on the testing pypi, 1.0.0 would still work on the production pypi. If anything goes wrong, we can still go to 1.0.1 there.

@mattytrentini sounds good, if you can announce it there also after we published an official release, that would be great!

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Oct 13, 2021

About newcomer friendliness: guess we could point to the counter.py example (or to the examples folder) from the README.

I guess it can be assumed that people who dive in as deep as using the ULP know how to use an esp32 (install python code on it, do python esp32 development), but do not know yet how to use our code.

counter.py is good as it is basically the simplest example code that actually does something visible.

@wnienhaus
Copy link
Collaborator

@mattytrentini Thank you for the great feedback and offer to help advertise this!

When you say "could potentially be friendlier", do you refer to the tone of the README, or the detail of it, or mostly that it lacks a clear "how to get started" section? The last part I will propose an improvement to, in line with what @ThomasWaldmann suggested above. But perhaps in your eyes there is more that we can do to make it friendlier?

@wnienhaus
Copy link
Collaborator

@ThomasWaldmann I'm fairly sure 1.0.0 would work on the production PyPi. So lets stick with the 3 part version number. Incrementing the last part of the version number, whenever we need to republish for some reason sounds "good enough for our purposes" (a phrase a friend of mine loved to use) 😄

Regarding a pointer to the examples from README, I will make a proposal via a PR.

I have also made a PR #56 for the setup.py part - making it upip installable. That is what I tested with test.pypi.org yesterday and subsequently on my ESP32. There is a question about including a file from another project, but I have more detail in the PR.

@mattytrentini
Copy link

When you say "could potentially be friendlier", do you refer to the tone of the README, or the detail of it, or mostly that it lacks a clear "how to get started" section?

Yes, mostly the latter part; how to get started. But also an overview. But now that #57 is opened I'll add more detail there. Thanks for trying to improve it!

@ThomasWaldmann
Copy link
Collaborator Author

OK, docs improvements have been merged, guess we can do a release soon.

@wnienhaus
Copy link
Collaborator

Yes, it looks like we can release soon. From what I see, the following are the next steps:

  1. Tag the repo with 1.0.0 (likely using the Github release functionality)
  2. Build and upload the py-esp32-ulp package to PyPi
  3. Announce

I wanted to document how to do step 2, so that you (@ThomasWaldmann) could do this with your credentials, as you are the owner of this repo/project.

However, while thinking about how to cut the process to as few steps as possible, I started considering to automate this process. This would then require only configuring the PyPI auth token as a repo secret once and thereafter simply doing Github Releases. I have already tested this in a test repo (https://github.com/wnienhaus/ujqtvms) and it works really well.

So I created an issue (#58) to automate the process and will spend a bit of time adapting the working solution from my test repo and make one more (hopefully last) PR for this.

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Nov 21, 2021

I had a look at the automatically built/released package file and added a MANIFEST.in so we are able to exclude some files which are committed (so setuptools_scm adds them to the auto-generated manifest), but which we do not really want to add to the pypi pkg:

  • .github/
  • .gitignore, .gitattributes

Currently also packaged (do we want to exclude that stuff from the pkg?):

  • docs/
  • examples/
  • tests/

On a PC/Mac dev machine guess it doesn't matter much, but how about when using upip directly on the esp32? Guess we only want to have the bare minimum in the package for that scenario?

@wnienhaus
Copy link
Collaborator

Hmm.. That was definitely not my intention. I believe I tested that when I first added setup.py and it only packaged files under esp32_ulp, which I understood came from the packages=["esp32_ulp"], line in setup.py. Because of that I decided to skip adding a MANIFEST.in because I got what I wanted from the automatic behaviour. So something changed and I wonder if switching to setuptools_scm is the difference (of course I should have checked what I got, but thank you for noticing).

I will add a MANIFEST.in and I think that we should only package the files under esp32_ulp, especially for the esp32. The rest, if needed can be gotten on a PC by cloning the repo.

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Nov 21, 2021

setuptools_scm is auto-generating the manifest ("in memory", not in the file), git-committed files are "in", everything else is "out".

see the comment i added in MANIFEST.in.

@wnienhaus
Copy link
Collaborator

wnienhaus commented Nov 24, 2021

Thanks. That makes a lot of sense. I have now also seen this quite clearly defined in setuptools-scm's documentation :)

I just sent a PR #62 for an updated MANIFEST.in which results in only the main source code being put in the package. I think we don't need (or want) other files on an esp32. (Not sure about examples. They might be nice to test that it works, but after that users would likely upload custom code anyway).

Let me know what you think.

@wnienhaus
Copy link
Collaborator

Thanks for merging #62. Looks like all is ready for release now.

Probably a release of a pre 1.0.0 (e.g. 0.1.2) would still be good, for ensuring that publishing to real PyPI works correctly. When I did this with a test project, I needed to first create an API token in PyPI, which had scope set to "Entire account", because for the first publish, the project didn't exist in PyPI yet. Once I had published the first release for that project, I then deleted the API token and created a new one, limited to the scope of only the specific project.

Do you want to do this first release to real PyPI (with a pre-release version) to "create the project" or should I do that, by manually publishing something under the "micropython-py-esp32-ulp" name, and add you as a collaborator (as I did for the project on TestPyPI)?

Once the whole flow works for the pre-release version, there should be nothing standing in the way anymore to release 1.0.0 (I can't think of anything).

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Dec 3, 2021

@wnienhaus can you give that pypi.org release a final manual test / review?

https://pypi.org/project/micropython-py-esp32-ulp/#files

(this was created automatically by the publish action after doing a github release of 0.1.5)

@ThomasWaldmann
Copy link
Collaborator Author

Next steps (if pkg is ok):

Do another dummy commit, tag 1.0.0, git push --tags, wait for CI to finish, github-release it.

The dummy commit is needed because having 2 tags on same commit confuses setuptools_scm - it might generate the wrong version number in such a case.

@wnienhaus
Copy link
Collaborator

wnienhaus commented Dec 3, 2021

Very nice! I just tested the package directly on a clean ESP32 (installed via upip). Works as advertised!
I also took a manual look over the files in the package and it all looks good to me.

About next steps, the dummy commit makes sense, but likely tagging should not be done with git, but rather let the Github release create the tag as part of the release?

@ThomasWaldmann
Copy link
Collaborator Author

Thanks for testing, so I'll create the release now.

AFAIK, it does not make a difference whether one tags with git (and then used that tag to create a release) or creates the release tag with github. Do you know more?

@ThomasWaldmann
Copy link
Collaborator Author

OK, so 1.0.0 publishing failed, because there already were some 1.0.x releases on test.pypi.org (and as that cancels the workflow, it didn't publish to pypi.org either).

To avoid these conflicts, we now have a 1.1.0 as first real release.

@ThomasWaldmann
Copy link
Collaborator Author

done! o/

@wnienhaus
Copy link
Collaborator

Yay! We made it! This needs some champagne 🍾 !
Thank you for all your effort reviewing and giving feedback! The result is better for it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants