firmware
firmware copied to clipboard
meshtastic
Genericise genericisable variant defines
This got a bit out of hand! It all started seeing comments from board makers, noting we had multiple overlapping variables for the same thing ... "FIXME: for some reason both are used in /src" " FIXME - we really should define LORA_CS instead" "[added for] Compatibility with variant file configuration structure"
Defines proliferated based on specific chip names, even though there was no different functionalities between chips. Re-use was minimal. For example, SX126X_CS and LORA_CS were commonly aliased everywhere, and used more or less interchangably.
Working theories:
- There are a minimum set of pins that tend to be used for LORA radios for which we need variables. These are LORA_* flags, and when a developer sets them in variant.h , they should expect relatively common and sane behaviour. The current set is: LORA_SCK, LORA_MISO, LORA_MOSI, LORA_CS, LORA_RESET, LORA_BUSY, LORA_DIO0
- If there's a need to do different things with these pins for a particular radio in our code we check USE_[RADIONAME] eg #if defined(USE_SX1268). See main.cpp for an example of how we do it right now for creating interfaces.
- For unique pins of a lora radio, make a new [RADIONAME]_* variable as we've always done!
I think that this direction will make it easier to add new variants, and particularly new variants with different lora radios. At the very least, it removes a few lines from most variant.h files ;)
So very tired.
CI / build-esp32 (tlora-v1) / build-esp32 (pull_request_target) - failed due to a timeout of github API, appears to be unrelated to code.
CI / build-esp32 (tlora-v1) / build-esp32 (pull_request_target) - failed due to a timeout of github API, appears to be unrelated to code.
Bravo on the work. I've poked CI to see if it'll complete that failure this time.
Is it possible to compile board firmware for master, and then for this PR, and verify that all the firmware files have the same checksum? This won't catch DIY boards but would be a very good indication of consistency
Some variant boards support more than one radio IC (still only one at a single time). As of my knowledge there are no multiple radio boards, but if someone wants to make one they should create a suitable system and not piggyback off the fact that their two radios use different ICs so use the different pin names, so I have no issues in this regard.
A complicating factor is that depending on the radio IC, different pins may be used in the same variant file, if the variant file supposed multiple different board types (433 MHz LilyGO vs 868/918 MHz LilyGO on same looking board), this aids flashing, so a single firmware file supports all versions of a specific board. This would need to maintain this flexibility. If both parts of the variant file are triggered, like in one of the comments, then this could redefine a pin, causing problems.
LORA_DIO1 can have multiple purposes, depending on the radio IC. If there are differences, these must all now happen once the chosen radio IC is settled on, not in previous phases, code will probably have to be modified.
There are presence checks if radioIc_pinName is defined, this affects the program logic, the same logic effect would have to happen before and after this PR.
Crucially, the new LORA_ pins must be used in the relevant parts of the firmware!
Sorry for all the work you put into this, but this will NOT work. Look at variants/tlora_t3s3_v1/variant.h for instance, this board is either shipped with SX1276, SX1262 or SX1280 and this firmware auto-detects which chip is actually used.
The chip specific macros are used to define a set of connected and functional pins for each of these boards, which clashes here for SX1262 vs. SX1280. They're both active at the same time during firmware build.
Same goes for other Lilygo radios, like the tbeam variants or the T-Echo.
Hey! Thanks for the review and taking the time to provide education too, much appreciated. This was still a worthwhile activity for me for learning the code base, so I don't mind :)
What do you think of flipping this idea - doubling down on the chipname variables and getting rid of the lora_* ones?
What do you think of flipping this idea - doubling down on the chipname variables and getting rid of the lora_* ones?
Not an easy one either. We just streamlined a few of these to replace the RF95 named variables :-)