Add type annotations

[jcerise]

Addresses issue #65

Adds type annotations across the file.

Items of note:

• I inferred types to the best of my ability in a few areas, any feedback on incorrect types is appreciated
• Bare asterisk usage in function signature triggers mypy to say that the signature is missing an annotation, not quite sure how to properly suppress that.

[tekktrik]

Awesome job with this, what a doozy! 😰

Feedback and comments attached, let me know if I can help explain anything!

[tekktrik]

Also, the bare asterisk signifies that any arguments following it, must be keyword-only, so it doesn't make sense to type annotate it since it's more of a sentinel character than actual argument. But I don't know how to argue with (or otherwise suppress) mypy! Maybe there is a suitable type annotation for it, but if so, we haven't been using it so no worries!

[jerryneedell]

I do hope this will get tested on hardware before merging. Normally I would be happy to do it, but I will not have access to my hardware for a free weeks. Good luck!

[jcerise]

Thanks so much for the feedback! I addressed everything as best I could. I saw another contributor say this earlier in a different PR, but this is legitimately the best first experience I have had with a project like this. I really appreciate the time and care taken to address this PR.

There's certainly some tribal knowledge that is now much clearer than when I started, and I learned a few things :)

I agree that testing on hardware would be good, I would be happy to jump forward and volunteer, but I started this at Pycon where there was hardware available, and unfortunately, I no longer have access to the hardware to test this. It is just type annotations though (aside from the requirements changes, and the new try/catch at the top) and should not (famous last words? I hope not :D ) affect the actual functionality of this library. However, I would probably feel better waiting on having this merged until it can be properly tested.

[jerryneedell]

@jcerise first of all, thank you for jumping in at PyCon to help with this. Don't let the comments of a curmudgeon like me dampen your enthusiasm ;-)
As I noted before, I'll defer to the other CircuitPython Librarians to comment and/or accept the "type" changes without testing. I don't fully understand what they are for or what impact they have on code size or execution.
I jumped in when executable code changes like the assert were added. I would argue that something like that clearly required on hardware testing.
If there are no changes to the code execution, again, I'll defer to others to make that call. I don't want to hold things up especially when I am unable to provide the testing.
My one concern is that the changes are adding to the code size enough to impact the feather_m0_rfm9x build of CIrcuitPython which includes this library as a "frozen" module. I would suggest that someone verify that it still builds with this PR. Again, I apologize that I am unable to do that myself at this time.

[tekktrik]

Big agree - thank you for jumping in to help at PyCon, @jcerise! Glad it's been an enjoyable experience - it should!

Type annotations are generally safe, but I would feel better building it if it's frozen in - I just never remember which libraries those are, so thank you @jerryneedell for mentioning that! I'll build it once it's ready to be safe.

I can take care of testing that, @jcerise, but if you're wondering what we mean by "frozen in", here's some information: https://learn.adafruit.com/building-circuitpython/adding-frozen-modules

Basically one build of the CircuitPython firmware has this cooked into it, so extra should be given to memory size (type annotations are ignored by the compiler I believe) and functionality (since this is cooked into firmware for specific boards, we get the benefit of the user not needed to provide this library, but it also means that any bugs will proliferate much more readily 😅)

[FoamyGuy]

Echoing what a few have said, thanks so much for working on this @jcerise!

My thoughts on a few things mentioned above:

Regarding suppressing mypy, I think we don't need to worry about it for now. We don't have the checks put into our CI yet so for the time being it's okay if it's still complaining about some things. When the time comes to add it to our CI checks we'll discuss the way we want to configure it which I assume will suppress any messages that we can't or don't intend to comply with.

regarding testing, we'll definitely test on hardware. I think I have some of the hardware to test this on. But it's also okay if we wait for a bit for jerry to return. I definitely believe that he's got more experience and hardware for testing more thoroughly than I.

We can do the frozen build test as well to make sure we won't run into size trouble. I'm not actually sure what impact the types have on final size of the mpy file. I think they may get stripped somewhere in the process but I'm not sure if the net impact is really 0 or maybe something more.

[tekktrik]

Just a few minor changes that I think got missed. After those, we can try building the firmware :)

[jcerise]

Thanks for the patience here. I realize I maybe should have chosen a smaller file for my first attempt, but with the guidance and discussion here, this has been great. My apologies for the silly mistakes above, those have been ironed out.

[tekktrik]

Nah, no worries, thanks for the quick turnaround!

[tekktrik]

Builds the Feather M0 RFM9x!

185536 bytes used, 2624 bytes free in flash firmware space out of 188160 bytes (183.75kB).
9844 bytes used, 22924 bytes free in ram for stack and heap out of 32768 bytes (32.0kB).

Converted to uf2, output size: 371200, start address: 0x2000
Wrote 371200 bytes to build-feather_m0_rfm9x/firmware.uf2

[tekktrik]

Small question for @FoamyGuy

[tekktrik]

Looks good to me! I'd approve it at this point, but it looks @FoamyGuy has it from here for testing, so it'll just get merged afterwards. Thanks @jcerise!

[FoamyGuy]

Looks good to me.

I tested several of the examples included in the repo successfully with https://www.adafruit.com/product/3231 on PyGamer, EdgeBadge, and Feather RP2040 all on 7.3.0-beta.2

[FoamyGuy]

Thanks again for working on this @jcerise!