Skip to content

1.0.0 milestone #49

@ThomasWaldmann

Description

@ThomasWaldmann
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.

Activity

wnienhaus

wnienhaus commented on Aug 9, 2021

@wnienhaus
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

ThomasWaldmann commented on Sep 27, 2021

@ThomasWaldmann
CollaboratorAuthor

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

added this to the 1.0 milestone on Sep 27, 2021
wnienhaus

wnienhaus commented on Sep 28, 2021

@wnienhaus
Collaborator

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

wnienhaus

wnienhaus commented on Sep 28, 2021

@wnienhaus
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

wnienhaus commented on Oct 7, 2021

@wnienhaus
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

ThomasWaldmann commented on Oct 8, 2021

@ThomasWaldmann
CollaboratorAuthor
  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

wnienhaus commented on Oct 9, 2021

@wnienhaus
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

ThomasWaldmann commented on Oct 9, 2021

@ThomasWaldmann
CollaboratorAuthor

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

14 remaining items

ThomasWaldmann

ThomasWaldmann commented on Nov 4, 2021

@ThomasWaldmann
CollaboratorAuthor

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

wnienhaus

wnienhaus commented on Nov 16, 2021

@wnienhaus
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

ThomasWaldmann commented on Nov 21, 2021

@ThomasWaldmann
CollaboratorAuthor

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

wnienhaus commented on Nov 21, 2021

@wnienhaus
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

ThomasWaldmann commented on Nov 21, 2021

@ThomasWaldmann
CollaboratorAuthor

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

wnienhaus commented on Nov 24, 2021

@wnienhaus
Collaborator

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

wnienhaus commented on Nov 28, 2021

@wnienhaus
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

ThomasWaldmann commented on Dec 3, 2021

@ThomasWaldmann
CollaboratorAuthor

@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

ThomasWaldmann commented on Dec 3, 2021

@ThomasWaldmann
CollaboratorAuthor

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

wnienhaus commented on Dec 3, 2021

@wnienhaus
Collaborator

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

ThomasWaldmann commented on Dec 3, 2021

@ThomasWaldmann
CollaboratorAuthor

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

ThomasWaldmann commented on Dec 3, 2021

@ThomasWaldmann
CollaboratorAuthor

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

ThomasWaldmann commented on Dec 3, 2021

@ThomasWaldmann
CollaboratorAuthor

done! o/

wnienhaus

wnienhaus commented on Dec 4, 2021

@wnienhaus
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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Participants

      @mattytrentini@ThomasWaldmann@wnienhaus

      Issue actions

        1.0.0 milestone · Issue #49 · micropython/micropython-esp32-ulp