Add Travis Continuous Integration to the repo

[peternewman]

Still a few bits to tidy up.

Which platforms should it compile on (e.g. Teensy too)?

[peternewman]

Looks like the Due failures are down to arduino/ArduinoCore-sam#31

[per1234]

This looks very nice @peternewman.

I notice you're using both spellintian and codespell. I have been using codespell for a while now and I find it very nice, but I have no experience with spellintian. I'm curious to know your reasons for using both these programs.

I don't think it's a good idea to use PlatformIO as the sole basis for compilation testing of official Arduino code. PlatformIO has significant differences from the Arduino IDE which often results in code that compiles on one not compiling on the other. For this reason, I think it is worth considering having a PlatformIO test in the CI to ensure compatibility with that platform. However, Arduino's primary focus must be compatibility with the Arduino IDE and Arduino Web Editor that the vast majority of Arduino users use.

I'm very much in support for adding CI testing to this repository and all of Arduino's libraries and I really appreciate your work towards that goal. I would like to see us settle on a standardized system that is used consistently across all the repositories. I would say the style demonstrated here:
https://github.com/arduino-libraries/WiFi101/blob/master/.travis.yml
would probably count as the closest thing we have to this currently in action.

I have been working to add some additional tests, which are somewhat similar to what you've done here (spellcheck, code formatting check). This was implemented in a few of the library repositories as somewhat of a demonstration and proof of concept. The decision was made that, before applying that same system to all of Arduino's libraries, we needed to have an official decision on how to do things. That discussion is here:
arduino-libraries/Arduino_MKRGPS#1

[sandeepmistry]

Hi @peternewman,

Thanks for taking the time to submit this, some comments:

• as @per1234 mentioned, we prefer to use arduino-cli
• I would prefer not to touch the upstream files in src/libmodbus/, as this will make syncing new versions of the external lib a bit more painful
• as of now, the main target is the Arduino SAMD21 type boards, inconjunction with the MKR RS485 shield

[peternewman]

• I would prefer not to touch the upstream files in src/libmodbus/, as this will make syncing new versions of the external lib a bit more painful

That's fine, I'll upstream them, I didn't realise you pull from upstream, I assumed it was a fork with mods.

• as of now, the main target is the Arduino SAMD21 type boards, inconjunction with the MKR RS485 shield

I started down this as a friend with a Due was trying to use the TCP version of the library (which seems to require the RS485 stuff regardless) and I thought I'd try and aim for some test driven development. I take it that means you're not targetting boards like the Due currently, but you'll accept patches?

[peternewman]

This looks very nice @peternewman.

Thanks

I notice you're using both spellintian and codespell. I have been using codespell for a while now and I find it very nice, but I have no experience with spellintian. I'm curious to know your reasons for using both these programs.

Somewhat historic I guess, one of the main things I contribute to goes into Debian amongst others, which therefore means a deb and lintian found spelling errors via spellintian, so I started running that as part of our CI. I later found codespell, and contribute to that too. They offer slightly different dictionaries and features (e.g. spellintian does multi-word detection, codespell does custom regex so it can check snake_case words individually. spellintian also does duplicate word checks, but they seem to be mostly false positives with Doxygen hence the allowed failure). They don't yet seem to disagree at least!

I also use codespell HEAD, so it's more bleeding edge, but also less bigger bangs when a new dictionary is released. The last release was three months ago, and before that seven months, where as new things are always going into the dictionary (and frequently appearing in my codebases).

I don't think it's a good idea to use PlatformIO as the sole basis for compilation testing of official Arduino code. PlatformIO has significant differences from the Arduino IDE which often results in code that compiles on one not compiling on the other. For this reason, I think it is worth considering having a PlatformIO test in the CI to ensure compatibility with that platform. However, Arduino's primary focus must be compatibility with the Arduino IDE and Arduino Web Editor that the vast majority of Arduino users use.

I think when I first tried CI for Arduino the CLI didn't exist (from the looks of things I was a week too early) so I didn't have much choice. I've also done some Teensy stuff and I'm unclear if that works via Ardunio CLI.

I'm very much in support for adding CI testing to this repository and all of Arduino's libraries and I really appreciate your work towards that goal. I would like to see us settle on a standardized system that is used consistently across all the repositories. I would say the style demonstrated here:
https://github.com/arduino-libraries/WiFi101/blob/master/.travis.yml
would probably count as the closest thing we have to this currently in action.

It sounds like I should wait until that's agreed for now (and perhaps open a separate PR with the typo fixes?

I have been working to add some additional tests, which are somewhat similar to what you've done here (spellcheck, code formatting check). This was implemented in a few of the library repositories as somewhat of a demonstration and proof of concept. The decision was made that, before applying that same system to all of Arduino's libraries, we needed to have an official decision on how to do things. That discussion is here:
arduino-libraries/Arduino_MKRGPS#1

That looks to have got a bit stuck, with no comments for a month...

[per1234]

Thanks for the information on codespell and spellintian. spellintian's multi-word detection feature sounds nice. My instinct would be to just pick the best of the two and only use that to keep things more simple, since it's not the end of the world if an occasional typo slips through.

Disregarding the Debian factor, since it doesn't apply here, which of the two do you think you would use if you could only use one of them?

I've also done some Teensy stuff and I'm unclear if that works via Ardunio CLI.

Note that there are two different official CLI options from Arduino. The Arduino IDE has had a CLI ever since 1.5.2:
https://github.com/arduino/Arduino/blob/master/build/shared/manpage.adoc
and that is used in several of Arduino's Travis CI configurations. There is also arduino-cli, which is a much more recent development. You can definitely install Teensyduino and use the Teensyduino CLI to compile sketches (though I have never done this in a Travis CI configuration):

arduino --verify --board teensy:avr:teensy36:usb=serial,speed=180,opt=o2std,keys=en-us /examples/01.Basics/BareMinimum/BareMinimum.ino

It does seem less likely that you could use arduino-cli to compile for Teensy boards. Even if it is possible, it will be challenging to install the Teensy boards core and toolchain, since it is not available in a Boards Manager package. Perhaps this will change once arduino-cli is integrated into the Arduino IDE (and thus into Teensyduino).

It sounds like I should wait until that's agreed for now

The only unresolved part of that discussion is about the code formatting check. Everyone is in agreement with using arduino-cli to do compilation tests of the library examples and there has been no disagreement to adding the spell check or Travis Buddy. So I think if you're motivated to get CI set up in this repository right now, you could just implement those parts, using the .travis.yml from https://github.com/per1234/Arduino_MKRGPS/blob/add-travis-CI-demo/.travis.yml as a reference. That would be a very valuable contribution. Once the code formatting thing is resolved we can add that in to the CI configuration.

However, I should say that I don't have any power to decide whether your PR will be merged or not. What really matters is what one of the developers, like sandeepmistry, thinks of it.

[peternewman]

Thanks for the information on codespell and spellintian. spellintian's multi-word detection feature sounds nice. My instinct would be to just pick the best of the two and only use that to keep things more simple, since it's not the end of the world if an occasional typo slips through.

Disregarding the Debian factor, since it doesn't apply here, which of the two do you think you would use if you could only use one of them?

I'm obviously a bit biased as I've contributed to codespell and not spellintian, but dictionary wise it seems like a no-brainer, 21k entries in codespell ( https://github.com/codespell-project/codespell/blob/master/codespell_lib/data/dictionary.txt ), versus only 8k in spellintian ( https://salsa.debian.org/lintian/lintian/blob/master/data/spelling/corrections ).

I'd broadly say spellintian seems more suited to documentation than code, codespell also has a LOT more options and hence flexibility in how it behaves, so I'd recommend going for that.

I've also done some Teensy stuff and I'm unclear if that works via Ardunio CLI.

Note that there are two different official CLI options from Arduino. The Arduino IDE has had a CLI ever since 1.5.2:
https://github.com/arduino/Arduino/blob/master/build/shared/manpage.adoc
and that is used in several of Arduino's Travis CI configurations. There is also arduino-cli, which is a much more recent development. You can definitely install Teensyduino and use the Teensyduino CLI to compile sketches (though I have never done this in a Travis CI configuration):

arduino --verify --board teensy:avr:teensy36:usb=serial,speed=180,opt=o2std,keys=en-us /examples/01.Basics/BareMinimum/BareMinimum.ino

It does seem less likely that you could use arduino-cli to compile for Teensy boards. Even if it is possible, it will be challenging to install the Teensy boards core and toolchain, since it is not available in a Boards Manager package. Perhaps this will change once arduino-cli is integrated into the Arduino IDE (and thus into Teensyduino).

Thanks for the info, I'll perhaps try and switch some stuff to arduino-cli first then and maybe look at moving the ones with Teensy at a later date.

It sounds like I should wait until that's agreed for now

The only unresolved part of that discussion is about the code formatting check. Everyone is in agreement with using arduino-cli to do compilation tests of the library examples and there has been no disagreement to adding the spell check or Travis Buddy. So I think if you're motivated to get CI set up in this repository right now, you could just implement those parts, using the .travis.yml from https://github.com/per1234/Arduino_MKRGPS/blob/add-travis-CI-demo/.travis.yml as a reference. That would be a very valuable contribution. Once the code formatting thing is resolved we can add that in to the CI configuration.

What about the common helper script, is that going ahead? That seems like the key part of this (and was part of the appeal of platform.io, is that then the .travis.yml basically says compile these examples against this matrix of boards, run a spellchecker (in this case ignoring the upstream folder) and the guts of how to do the various things like installing libraries and the spellchecker is hidden away in a common codebase.

However, I should say that I don't have any power to decide whether your PR will be merged or not. What really matters is what one of the developers, like sandeepmistry, thinks of it.

Okay thanks @per1234 . @sandeepmistry are you broadly in favour if I do as suggested above?

[per1234]

What about the common helper script, is that going ahead?

Good question. From the discussion in arduino-libraries/Arduino_MKRGPS#1, the main area I identified as needing to be moved to a common helper script was the code formatting check commands, which I did (and is not relevant to this PR). So I had considered that issue to be resolved.

However, your comment made me remember that there was a recent discussion via a private channel where it was proposed that some of the commands used in the compilation test might also be added to a common script. I can see benefits to that, especially after some recent changes to the way arduino-cli is installed that broke several of Arduino's CI configurations in multiple ways. It is convenient to be able to make a change to a single script to fix that, rather than having to do it 75 times over again (as would be the case once we have added CI to all of Arduino's library repositories).

So I went ahead and created a script with these compilation test helper functions:
https://github.com/per1234/ci-resources/blob/master/compilation-test.sh
and updated my demonstration CI configuration to use them:
https://github.com/per1234/Arduino_MKRGPS/blob/add-travis-CI-demo/.travis.yml

I should warn you that this is new and I have not received any feedback on that work from anyone at Arduino so this may only be a preliminary implementation. I welcome any contributions or feedback for improving or extending that work.

I'm not sure if it's necessary to do the same for codespell, since those commands are already so simple. I suppose functionalizing it could allow us to easily switch to a different spell checking application if we found a significantly better one at some point. However, there are things like language: python that you can't do via a script so we might get blocked on that anyway.

[sandeepmistry]

So we're starting to adopt GitHub CI/CD workflows instead of Travis CI, some examples:

• Use Github workflows to compile examples and spell check (replaces Tavis CI) ArduinoMqttClient#16
• Use Github workflows to compile examples and spell check Arduino_OAuth#1
• https://github.com/arduino-libraries/ArduinoBLE

If you'd like to try to follow that model here, please let us know.

[aentinger]

Hi @peternewman 👋 As @sandeepmistry mentioned we are transitioning to GitHub Action based CI. I'm aware that this is rendering your effort obsolete for which I'm very sorry but I hope you at least learned something from doing this.