adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.07k stars 1.2k forks source link

Standardize on RFM lora module pin naming. #7830

Closed reapzor closed 6 months ago

reapzor commented 1 year ago

Lora modules seem to be getting more popular in circuit python boards (also the ability to expose esp32 boards that had these in the past is now a thing), and I'm looking to add a couple esp32 boards that have lora modules soldered on them that work with the adafruit rfm9x library.

I noticed recently that theres a new rp2040 feather with lora from adafruit. Since the Adafruit Feather RP2040 with RFM95 is new and awesome it would be nice for documentation on how to use the board to have good pin names up front. This new feather uses MP_QSTRRFM, while the feather M0 uses MP_QSTRRFM9X. When searching for "MP_QSTR_RFM" there are what seems like 3-4 naming variants on the pin names ranging from RFM to RFM9x to RFM95w to RFM69 (probably ok since that would be a different library too) https://github.com/search?q=repo%3Aadafruit%2Fcircuitpython+MP_QSTR_RFM&type=code

The adafruit documentation for this library here https://learn.adafruit.com/adafruit-rfm69hcw-and-rfm96-rfm95-rfm98-lora-packet-padio-breakouts/circuitpython-for-rfm9x-lora and here https://docs.circuitpython.org/projects/rfm9x/en/latest/ would be nice to reference more standard pin names. I assume one for the new feather rp2040 with rfm module may come out eventually as well.

This is an ask to start thinking about a standardized naming convention for built-in lora modules on circuit python boards so that the documentation can be more clear on what pins to use, especially now that it has expanded beyond the feather m0 into other adafruit boards. Not sure how to best handle backward compatibility, but it seems like this could benefit from a strategy going forward at least for new boards.

tannewt commented 1 year ago

@kattni has been working on the new board defs.

dhalbert commented 1 year ago

I think just RFM_* would be OK, as the simplest, assuming there wasn't a board with more than one module :) . It also makes the examples more uniform.

We can always add multiple names for a single pin. On the M0 boards, this takes valuable space, but those boards are difficult to use with CircuitPython because they have such limited RAM. The RP2040 board is what we would recommend going forward.

@kattni Do you have an opinion here?

kattni commented 1 year ago

Whatever I did for the Feather RP2040 RFM board definition is what we should use moving forward. IIRC, those pins show up as RFM_*. If we eventually end up with a board with multiple modules, we can address how to handle that.

dhalbert commented 1 year ago

Add guidance on these pins names to the "how to add a board" Learn Guide

dhalbert commented 1 year ago

Current pin name prefixes below. It is also the case that the individual pin names don't match. For instance, RFM9X_D0 vs RFM9X_DIO0 vs RFM_IO0, etc.

I am not sure it is worth making an incompatible change to regularize these. We could add uniform aliases across all the boards in 9.0.0 and remove the old non-uniform names in 10.0.0. Pushing this forward to 9.0.0 for now.

dhalbert commented 7 months ago

@jerryneedell Do you have an opinion about this? I am pretty reluctant to add aliases to the M0 boards, because they are so tight on space. Maybe we should just make up some guidelines going forward for the best pin names. If you have suggestions for that, I'd be interested as well.

jerryneedell commented 7 months ago

How much space does an alias add to the M0 build? I like the generic RFM* names used on the RP2040 and I think it is unlikely that multiple modules will be present on the same board. Even if they were, then that would warrant some alternative naming for that case. Using RFM would make it a lot easier to migrate between boards and to write guides and examples for multiple boards. It is more like the way displays are handled with TFT_. Changing the M0 names will break a lot of existing code…. If it is only a few bytes, the alias may be worth it. I don’t see how the M0’s will be able to be maintained going forward if the RFMxx libraries ar to be able to evolve. On that topic, and we probably should start a new issue to discuss it, perhaps we should consider freezing the RFMxx libraries and create a new library going forward with a new name and expanded capabilities.

dhalbert commented 7 months ago

Adding the RFM aliases takes 84 bytes extra on Feather M0 RFM69. We recently gained a lot of space by switching to shorter error messages on these smallest builds.

    { MP_ROM_QSTR(MP_QSTR_RFM_D0), MP_ROM_PTR(&pin_PA09) },
    { MP_ROM_QSTR(MP_QSTR_RFM69_D0), MP_ROM_PTR(&pin_PA09) },
    { MP_ROM_QSTR(MP_QSTR_RFM_RST), MP_ROM_PTR(&pin_PA08) },
    { MP_ROM_QSTR(MP_QSTR_RFM69_RST), MP_ROM_PTR(&pin_PA08) },
    { MP_ROM_QSTR(MP_QSTR_RFM_CS), MP_ROM_PTR(&pin_PA06) },
jerryneedell commented 7 months ago

I suppose the other way to look at this is to say that leaving it as is only means adding a few lines to be commented in/out of any examples. Perhaps that is the better way to go to avoid the inevitable issues with running out of space for every change to 9.x.x builds. 84 bytes is not trivial.... As noted, going forward standardized names should be used but it may not be worth losing back at the M0 boards.

Regarding the examples, we still will likely include examples of how to use them with breakouts as well. A special note for the M0 boards is not a big deal.

dhalbert commented 7 months ago

The older names would be dropped in 10.0.0. But we'd need to change several guides and examples.

Another change I see is to name the I/O pins DI0, DIO1, etc. uniformly. Those are the names used on both the SXnnn datasheets and the RFMnn module datasheets. Then they don't get confused with the regular D0, etc. pins.

For any future boards, I'll try to make sure the names are consistent with the guidelines described so far:

Also the Arduino pin names are not consistent either.

I think I'll just give up on this for now and consider it for a later 9.x.x release or later. It is not urgent, and is not a requirement for 9.0.0.

dhalbert commented 6 months ago

A hew page in https://learn.adafruit.com/how-to-add-a-new-board-to-circuitpython about pin name conventions is in progress. Closing this as resolved by that.