Github workflow to compile example sketches on RPi Pico and ESP32. - #231
Conversation
|
|
||
| #ifdef USE_MEGA_SERIAL1 | ||
| // Use hardware Serial1 on pins 18 (TX1) and 19 (RX1) | ||
| #define serialUserPort Serial1 |
There was a problem hiding this comment.
Here the comment should be updated to list the pins used on Mega and ESP32.
There was a problem hiding this comment.
Done. Although ESP32 seems to be flexible here and can use any pins.
There was a problem hiding this comment.
But does it uses defaults pins if none is explicitely defined in the Serial1.begin()? I assume it is ESP32 variant dependent.
Also, about pins, does the arduino RP2040 platform supports A0 - A6 definition? In the examples, the A0 - A4 are used as digital inputs.
const byte SWITCH[] = {A0, A1, A2, A3}; // Module Switch takes input to 0V.There was a problem hiding this comment.
Hmm. Here we come into a dilemma. I don't know enough about ESP32 and I don't have any board here. I just wanted a sketch that compiles for an ESP32 platform. And it does as it stands now. This also means that the sketch is not a working example for ESP32 boards. How far shall we go in supporting something we cannot verify? How much #ifdef can we tolerate? My thinking is that any user of these examples has to do their homework and define the pins they use. OK, I should make that clear in documentation and comments in the sketches.
No, the ESP32 board definitions don't have default pin numbers. The default pins are -1 and -1. #85
Regarding the analogue pins A0-A3, yes they work on a RPi Pico. Note that this board only has 4 analogue pins.
And yes, these pins are used as digital inputs. The reason we picked these pins is that these are the remaining pins on a Arduino Uno/Nano.
There was a problem hiding this comment.
Whilst the RP2040 has 4 analogue inputs (A0 - A3 on GPIO26 - GPIO29), the Pico only makes A0 - A2 available. The same goes for Pico2, which uses the RP2350a processor. The RP2350b has more GPIO including 8 analogue.
There was a problem hiding this comment.
Personally, I would stick with AVR only examples for the default workflow, and use a separate workflow for each platform using just the VLCB_SerialGC_empty example to check that the VLCB library builds correctly on that platform.
Interesting thought. Would certainly speed up things. But hey, I am not paying for the compile time. ;)
I think I prefer to stick with one workflow:
- Only one workflow to maintain.
- There are some complexities in the other examples that should be tested. They shouldn't cause problems but I want to be on the safe side.
There was a problem hiding this comment.
I think there are options, but it is difficult to make a choice:
- Provide with the library a set of examples that would work straight on selected platforms/device
This means that each example should be adapted to support every selected platform, i.e. AVR, RP2040, ESP32, etc.
Because of the differences, I am affraid this would lead to many #ifdef/#endif constructs per example, making them hard to read/understand for a beginner.
Alternatively, examples for a specific platform could be placed in separate folders, by platform name like:
+---examples
| +---MDF
| +---avr
| | +---VLCB_SerialGC_1in1out
| | +---VLCB_SerialGC_4in4out
| | +---VLCB_SerialGC_4in4out_slot
| | +---VLCB_SerialGC_empty
| | \---VLCB_SerialGC_SerialUI_empty
| +---esp32
| | +---VLCB_SerialGC_1in1out
| | +---VLCB_SerialGC_4in4out
| | +---...
| \---rp2040
| +---VLCB_SerialGC_1in1out
| +---VLCB_SerialGC_4in4out
| +---...
But this means duplicating the examples and maintaining them separately. Maintaining them won't be more difficult than in the previous case, I believe. At least the code should be more readable.
If another platform is needed, just add a folder with the dedicated examples and update the workflow.
This would apply as well to the VCAN2515 and similar for their own examples.
- Only provide complete examples for AVR and use minimal examples for the other platforms
This was my previous idea to limit the support to a well known base: "If it works on AVR, it should work on anything else". The "examples" for the other platforms would only serve one purpose: check that the library builds in the platform environment. This would complement the native tests executed via CMake. The native tests won't be able to detect platform specific issues.
Having separate workflow is not really adding complexity. It just makes the outcome available faster without having to wait for the others. If the esp32 build fails, you will know quickly instead of having to wait for the big workflow to complete and check which one passed and which one failed. Also the platform specific workflow could check against multiple variants, for example ESP32-S2 (Xtensa) and ESP32-C6 (RISC-V).
There was a problem hiding this comment.
Having separate workflow is not really adding complexity. It just makes the outcome available faster without having to wait for the others.
Using the "matrix" feature we get separate jobs that run in parallel. So works the same way as separate workflows. And with the strategy flag "fail-fast: false" these jobs don't abort if one of them fail.
I still like to have everything in one workflow. We could have several jobs in one workflow. The only difference is that the job status is not reported until the whole workflow finishes. I can live with that.
Also the platform specific workflow could check against multiple variants, for example ESP32-S2 (Xtensa) and ESP32-C6 (RISC-V).
I have not much knowledge about the ESP32 boards. Didn't even know they used different processor families. Give me a list of distinct ESP32 variants that would be useful to compile for and I'll add them.
There was a problem hiding this comment.
Thanks for those options.
I agree with your point about #ifdef noise in the code. So far there is only one instance, in VLCB_SerialGC_SerialUI_empty.ino .
A directory structure is interesting. Yes, it adds duplication. So far I prefer the #ifdef over this duplication.
Limiting the examples for other platforms may a good way to reduce duplication.
My thinking is that the single #ifdef we have now is acceptable. But your options shall be reconsidered when there is a need for more #ifdef's.
There was a problem hiding this comment.
As long as the #ifdef noise is limited, I am OK with single worklow and current examples. It would still be possible to move to another structure if need arises.
Extend the Arduino Firmware Build workflow to compile the example sketches for AVR, RPi Pico and ESP32 platforms.