nRF24 / RF24

OSI Layer 2 driver for nRF24L01 on Arduino & Raspberry Pi/Linux Devices
https://nrf24.github.io/RF24
GNU General Public License v2.0
2.23k stars 1.02k forks source link

Adding a CI workflow for compiling examples with Arduino CLI #665

Closed 2bndy5 closed 3 years ago

2bndy5 commented 4 years ago

Huge thanks to @per1234 for giving me a solid starting point on this issue!

Currently the RF24 library lists all boards as compatible in the library.properties file. We can reduce/grow the list of tested boards as we go, but I just wanted to focus on the AVR architecture first. Other platforms that require manual addition to the Arduino IDE's board manager (esp and samd) can be added as the GitHub Action that I'm using supports this feature, and many others like specifying platform variants (i.e. boards under the same name that have different CPUs).

So, here is the working file I'm using to address this issue.

Specifying Boards

The boards to compile against are specified in the workflow under the matrix named "fqbn". Each board uses the form: <vendor>:<arch-type>:<board-name> where vendor is arduino, arch-type is avr, and board-name is found in the boards.txt file (found in the Arduino IDE's "hardware/vendor/arch-type[/version]" folder) that prefixes the board options. So the Arduino Uno is specified as: arduino:avr:uno

requesting boards -- REVIEW THE MATIX FIRST TO SEE IF IT'S ALREADY ADDED

Feel free to post on this thread the boards you think should be tested (for now), but please use the format <vendor>:<arch-type>:<board-name>. If its a board that needs installing to the Arduino board Manager, please post the source-url of the git repo as well (more on that when I investigate further).

2bndy5 commented 4 years ago

I was thinking about adding platformIO CI workflow (under a separate issue), but the compilations seem to output warnings pertaining to errors in the specified core (e.g. LITTLE_ENDIAN redefined for the SAMD core).

2bndy5 commented 4 years ago

@per1234 How do I specify the SpenceKonde core with the arduino-compile-sketches action? I've added a separate job just for testing the rf24_ATTiny folder in the examples... Here's what isn't working for me so far:

  attiny:
    runs-on: ubuntu-latest

    strategy:
      fail-fast: false

      matrix:
        fqbn:
          - ATTinyCore:avr:attinyx4
          - ATTinyCore:avr:attinyx4opti
          - ATTinyCore:avr:attinyx4micr
          - ATTinyCore:avr:attinyx5
          - ATTinyCore:avr:attinyx5micr
          - ATTinyCore:avr:attinyx8
          - ATTinyCore:avr:attinyx8opti
          - ATTinyCore:avr:attinyx8micr
          - ATTinyCore:avr:attinyx7
          - ATTinyCore:avr:attinyx7opti
          - ATTinyCore:avr:attinyx7micr
          - ATTinyCore:avr:attinyx61
          - ATTinyCore:avr:attinyx61opti
          - ATTinyCore:avr:attinyx41
          - ATTinyCore:avr:attinyx41opti
          - ATTinyCore:avr:attinyx41micr
          - ATTinyCore:avr:attiny43
          - ATTinyCore:avr:attiny828
          - ATTinyCore:avr:attiny828opti
          - ATTinyCore:avr:attiny1634
          - ATTinyCore:avr:attiny1634opti
          - ATTinyCore:avr:attinyx313

    steps:
      - name: Checkout
        uses: actions/checkout@v2

      - name: Compile examples
        uses: arduino/compile-sketches@main
        with:
          platforms: |
            - source-url: "https://github.com/SpenceKonde/ATTinyCore.git"
            - name: "ATTinyCore:avr"
            - version: "latest"
            - source-path: "./avr/"
          sketch-paths: |
            - examples/rf24_ATTiny
          # - examples/rf24_ATTiny/rf24ping85
          fqbn: ${{ matrix.fqbn }}

which outputs the error Error during install: finding platform dependencies: package ATTinyCore not found. I've also tried other options (for name: and fqbn:) that didn't work. I guess I'm having trouble identifying the correct VENDOR:ARCHITECTURE to specify or maybe the platforms install info.

per1234 commented 4 years ago

Here's the problem:

          platforms: |
            - source-url: "https://github.com/SpenceKonde/ATTinyCore.git"
            - name: "ATTinyCore:avr"
            - version: "latest"
            - source-path: "./avr/"

The platforms input takes a string (thus the need for the | in platforms: | that is a YAML document defining a list of platform definition objects. Each list item is a definition of a separate platform dependency. So you need to put the entire platform definition object in a single list item:

          platforms: |
            - source-url: "https://github.com/SpenceKonde/ATTinyCore.git"
              name: "ATTinyCore:avr"
              version: "latest"
              source-path: "./avr/"

The platform dependency definition above only installs the platform. If you were using the Arduino IDE, that would be fine because it comes with the AVR toolchain bundled for the bundled copy of Arduino AVR Boards (arduino:avr). But Arduino CLI doesn't bundle anything, so you will be missing the AVR toolchain needed by the ATTinyCore:avr platform. An easy way to get that toolchain is to do a Boards Manager installation of the arduino:avr platform as well, even though you aren't compiling for any of the boards of that platform:

          platforms: |
            - source-url: "https://github.com/SpenceKonde/ATTinyCore.git"
              name: "ATTinyCore:avr"
              version: "latest"
              source-path: "./avr/"
            # Install Arduino AVR Boards platform to get the AVR toolchain
            - name: "arduino:avr"

I guess you can now see why it's important for the platforms input to be able to take a list of platforms!

per1234 commented 4 years ago
          platforms: |
            - source-url: "https://github.com/SpenceKonde/ATTinyCore.git"
              name: "ATTinyCore:avr"
              version: "latest"
              source-path: "./avr/"

There is a potential problem with this approach that's fairly specific to the SpenceKonde platforms.

The approach SpenceKonde uses for the Boards Manager releases is to use the archive files automatically generated by GitHub for every Git ref as the platform archive. This requires the platform to be in the root of the repository. However, that's not very convenient for manual installation, since it would require the user to create the architecture folder (e.g., {sketchbook folder}/hardware/ATTinyCore/avr/). So the ATTinyCore repo has the platform under the avr subfolder, but SpenceKonde creates a special tag on every release that has the platform in the root of the repository (example), which would be incompatible with your source-path: "./avr/" configuration.

The version: "latest" in your platform definition causes the newest tag to be used. It looks like SpenceKonde usually does the tag formatted for manual install first, then the one for the Boards Manager archive second, but I don't know whether that order is followed strictly. I can see some examples to the contrary in the SpenceKonde/megaTinyCore repo.

2bndy5 commented 4 years ago

You pointed out the potential problem just as I had realized it the hard way! My intention was to avoid pulling untested commits to the master branch by specifying the latest release. I did not notice that SpenceKonde had 2 releases per iteration. Furthermore I specified the v1.4.1 archive's zip asset and directed the source-path to ATTinyCore-1.4.1 directory which is the archive's root directory (which didn't work). Apparently this 1st directory is ignored because the following compiles using the SpenceKonde core (no source-path option specified):

          platforms: |
            - source-url: "https://github.com/SpenceKonde/ATTinyCore/archive/1.4.1.zip"
              name: "ATTinyCore:avr"
            - name: "arduino:avr"
          sketch-paths: |
            - examples/rf24_ATTiny

I also seem to be getting easily confused about how the YML syntax is used, specifically for this action's context vs the entire workflow's context. The action's docs about "needs to be a YAML formatted list" was rather vague for me (Wikipedia didn't seem to help clarify this).

per1234 commented 4 years ago

My intention was to avoid pulling untested commits to the master branch by specifying the latest release.

If you like, you can just install ATTinyCore via Boards Manager:

          platforms: |
            - source-url: "http://drazzy.com/package_drazzy.com_index.json"
              name: "ATTinyCore:avr"
            - name: "arduino:avr"

That will get you the latest release, or you can pin a specific release with the version field.

The repository and archive download dependency source options are more commonly used when you do want to test against an unreleased version of the project, or when the project is not available via Boards/Library Manager. But the action is intended to allow for any use case, so it should definitely do what you're going for here.

ATTinyCore-1.4.1 directory which is the archive's root directory (which didn't work). Apparently this 1st directory is ignored

I think I have documented this behavior here: https://github.com/arduino/compile-sketches#archive-download

I also seem to be getting easily confused about how the YML syntax is used, specifically for this action's context vs the entire workflow's context. The action's docs about "needs to be a YAML formatted list" was rather vague for me.

I agree that there is a lot of room for improvement in the documentation and welcome any input on that, or PRs. The approach of using strings that define a YAML document as the inputs of the action is kind of confusing if you think about it. My hope was that people will not really think about that and just say "OK, I can see from the example that have to put this funny | thing in, but after that the inputs work just like the rest of my workflow". So this is why I haven't attempted to actually provide an explanation of how the inputs work, but perhaps that's not the right approach.

(Wikipedia didn't seem to help clarify this)

The documentation of YAML is really not very easy to understand. It's ironic because, to me, the whole point of YAML seems to be to be as simple and syntax-free as possible, but if you go to the official website looking for basic information, there is none to be found!

I probably don't use the right terminology: "sequence of mappings".

I very much welcome any feedback, suggestions for improvement, PRs, etc.

2bndy5 commented 4 years ago

Default: root folder of the archive

This part made me think I had to direct source-path to the folder ATTinyCore-1.4.1 located at the archive's root.

My hope was that people will not really think about that and just say "OK, I can see from the example that have to put this funny | thing in, but after that the inputs work just like the rest of my workflow".

Coming from an English tutor's background, its best to not assume things when writing anything (this is a common hardship among readers of whitepapers and other academically reviewed material). The docs are fine for the most part, but remember that you could too smart for any newcomer's limited understanding (myself included -- you're amazingly impressive). They could use more example snippets that don't involve arduino:avr, and it would help a lot to have a place that explains how to determine what the Vendor and Arch-type fields are (more emphasis on the vendor part). I had to google how to find the fqbn identification which kinda led me to this added confusion about the Vendor/Arch-type.

You'll notice that my doc additions to this repo (as well as the ones I wrote for my CircuitPython-nRF24L01 library) are painfully explanatory. I think Markdown also has a subtitute-replace syntax that would help with what might feel like unneccessary repetition.

I want to get this repo squared away into a culminated PR that addresses the 16 issues I've raised about it. Then I'll try and do a PR about the compile-sketches action, as I'm sure I'll be in a much more experienced position to talk about it (thanks to you 💯 )

2bndy5 commented 4 years ago

I've decided to not include the adafruit-samd core into this workflow as the pins 7 & 8 (used in examples' instantiating args for RF24 objects) are not always available on these boards. I know I could do n #ifdef in the examples, but that just feels like extra clutter. I'll probably reach the same conclusion about the Espressif-arduino core, but I haven't to looked into it yet.

BTW, I've been testing all the new examples on the 2 adafruit samd-based boards. 😄