Add files needed to compile Ethernet library

[jgfoster]

We tried adding the Ethernet library and found that some pieces were missing. This adds the missing pieces and a test to show that we have enough to work on Ethernet separately. The test doesn't implement Ethernet, but shows that the pieces needed are now provided. Comments and suggestions are welcome.

[ianfixes]

Is there a reason that the changes to SampleProjects/TestSomething/.arduino-ci.yml wouldn't go in misc/default.yml?

[jgfoster]

The goal of the change was to add the define for ARDUINO=10813 to get the Ethernet code to run (see #176 & #179). When I chose the location, I was trying to keep my changes small and isolated. It seems that if it were added to misc/default.yml then we might as well just hard-code the value 10813 instead of 100 (and allow people to override it as they wish). My hesitancy is that it will then affect existing users whose code might be relying on the default of 100.

But most of my hesitancy is because I don't understand what I'm doing very well, so reducing the impact of my changes seemed prudent!

[ianfixes]

Aah, I think I see the significance of this, despite the conversation being spread out across a few issues.

I think the root issue here is going to be #185 so I'll focus on that next and see if it simplifies this.

[jgfoster]

You are right that this is a bit confusing because of the path I took that spread it across a few issues. While upgrading to 1.8.13 might reduce some of the issues I'm not sure that it would completely resolve the underlying issue. In another conversation you discussed the challenges of keeping an external library in sync with the core testing framework. On the one hand we like to have them separate (we rightly decided that we should not add all external libraries to Arduino CI), but we need to find out quickly if a change in the core framework breaks a dependent library. Does that mean creating a sample project that includes the external library? Should this have been in a new SampleProject directory rather than added as a (huge) .cpp file to TestSomething?

[ianfixes]

There's a question you're implying here that is a good one, and one that I meant to reply to sooner. I'd phrase it in my own words like this:

I have a library that uses preprocessor directives to behave differently for different versions of the Arduino IDE. How do I structure my project to be able to test both code paths with Arduino CI?

I think the answer to this is to expose the value of the ARDUINO macro to the command line, so you would call the arduino_ci_remote.rb command several times with each of the values you'd like to test. Does that sound workable? I can create an issue for that if so to track it.

There are a lot of really great features being requested here at the same time and I apologize for the delay -- I'm trying to make sure that I'm adding them in a way that keeps things coherent.

[jgfoster]

I'm not sure my implied question was quite so insightful, so it is good that you rephrased it. As you phrase it, it does cover one of the issues. But this PR really has several issues:

• This adds or updates Client.h, IPAddress.cpp, IPAddress.h, Print.h, Printable.h, Server.h, and Udp.h. That is a lot of code. You should be concerned about where I got it and how it can be tested!
• This adds SampleProjects/TestSomething/test/ethernet.cpp, a 3210-line file that doesn't do much except confirm that the files added above actually compile (which was my goal!). The real work testing different external Ethernet libraries will be done with those libraries. In thinking more about this I wonder if I should create a brand new SampleProjects/Ethernet that has various files but only tests the test file. This would reduce the clutter on TestSomething.
• Then there is the change to add ARDUINO=10813 and modify lib/arduino_ci/cpp_library.rb to support different values for the macro. The remainder of this comment addresses that issue.

As to the ARDUINO macro, there could be value in being able to test different code paths, but I'm afraid that is not really a satisfactory solution (for the same reason my approach of adding it to .arduino_ci.yml is probably wrong).

The "real" Arduino libraries have the ARDUINO macro for a reason: the behavior of the core classes changes with different releases. You are "mocking" some of the core libraries, and your mocks will tend to conform to a particular version of the real libraries. If you allow changing ARDUINO then you are implying that you can mock different versions of the core libraries, which I think is beyond the scope of what you are doing.

I think it would be better for you to upgrade Arduino CI periodically, especially when someone (like me!) needs to move from 100 to 106. But once you've done the upgrade, it seems that other libraries will only be tested against the version provided by the mocks. It seems to me that allowing testing of other mock versions would be best supported by using older versions of Arduino CI. (As I'm writing this, I'm wondering if Arduino CI should have versions that follow Arduino, with a custom extension; so the next would be 1.8.13.ci.0. That doesn't exactly follow Semantic Versioning, but does convey something quite useful!).

But then I'm on a (relatively) new project and I like to be on the "bleeding edge" of new versions, so I don't have as much concern for those who want to be able to use a new version of the testing framework to test old libraries.

By the way, how do you feel about a new issue deprecating arduino_ci_remote.rb and renaming it arduino_ci.rb? I think you said earlier that the difference you expected never materialized.

In any case, I appreciate your responsiveness and respect the concern for keeping things coherent. I'm pretty excited about being able to build an application that has thorough testing.

[ianfixes]

A few things to respond to

I think it would be better for you to upgrade Arduino CI periodically

Yes, if there was a guaranteed version of arduino_ci per IDE version, this would be a perfect way to simply iterate through downloadable versions of the IDE and launch the corresponding CI script version, no matter which combination(s) a project wanted to test. The ARDUINO macro would be set appropriately in all those cases.

Ethernet to SampleProjects/Ethernet

In truth I've never used an Ethernet library with an Arduino project, so I'm not familiar with whether the library here represents a native capability like SPI or HardwareSerial, or requires a shield of some kind. But based on the way you're describing it, it sounds like you might be right to move it to SampleProjects if it's structured like any other standalone library.

arduino_ci_remote.rb -> arduino_ci.rb

I think we should open a separate issue to track this.

Apologies for the delay, it's been quite a busy week

[ianfixes]

The other thing I'm noticing here is a lot of variation in the licenses prepended to those files. This project overall is apache2 licensed, and I'm not sure how that translates to accepting PRs that are labelled otherwise.

[jgfoster]

You are probably right about the license concern. What source do you use for the mock?

[ianfixes]

What source do you use for the mock?

I'm not sure what this means, I wrote all the mocks by hand to match the public APIs I could find in the Arduino documentation.

[jgfoster]

I see. I thought you had taken an existing set of files and replaced certain parts (much as I did).

[jgfoster]

This depends on #197 (and helped in its development!).

This is a major rewrite and relies on an import of the Ethernet library and provides brand-new, hand-crafted stubs for the required files (rather than bringing in the full copy from elsewhere (with the license issues).

Now that we can compile the Ethernet library, we can start to work on actually testing it (and making it testable.

[jgfoster]

I see that the appveyor test failed because a header file was not found. This works when I run locally, and I don't understand what is different. Any advice would be appreciated!

[ianfixes]

a header file was not found

I'm skimming this for now, and my first guess is that it has something to do with pulling in Ethernet as a dependency in the Libraries/ directory. I'm looking here: https://github.com/Arduino-CI/arduino_ci/blob/c85e1a083bde76f80d2750f8ef07caee0a3f769a/SampleProjects/NetworkLib/scripts/install.sh

Maybe locally you have this file in your Arduino Libraries dir, but my initial reaction (without testing this) is that your install script is not getting the Ethernet library to that location.

[ianfixes]

In general, this PR looks great. If you can add minimal unit tests for the new classes (just to ensure that the compiler is aware they exist), this will be ready for merging.

[jgfoster]

I think we have some basic tests and the library is properly installed. It should be ready for another review!

[ianfixes]

This seems like a bug in the core library, was it not able to fetch this by specifying a dependency in .arduino-ci.yml?

[jgfoster]

The dependency in .arduino-ci.yml would bring in the official library (code here?). But for the purposes of this test in Arduino Ci, we'd like to test against our fork of the Ethernet library, since it is our fork that has arduino_ci support.

[ianfixes]

Unless I'm missing something, this behavior of "install a standard library if a local one does not exist" is also implemented in the core library:
https://github.com/Arduino-CI/arduino_ci/blob/master/exe/arduino_ci.rb#L170-L173

In other words, if you manually install your own ethernet library locally, then listing a library of the same name in .arduino-ci.yml will not cause the CI runner to overwrite it. The check is simple directory existence: https://github.com/Arduino-CI/arduino_ci/blob/master/lib/arduino_ci/arduino_cmd.rb#L238-L240

So I think this entire script and the reference to it can be deleted

[jgfoster]

The comment does not do a good job of explaining what I want, so I've revised it.
When we are running the CI tests (on Travis, Appveyor, or GitHub Actions), we won't have the Ethernet library present at first. We don't want arduino_ci to install it because it will install the "standard" one and we want the "custom" one (my bad on the comment). But, I am running the CI tests on my development machine, I don't want it replacing my existing (development) Ethernet library. So, unless .arduino-ci.yml can specify a source (like a Gemfile), then I need the custom script. At least I think so! Does that make more sense?

[jgfoster]

While the purpose was to use our custom Ethernet library (the reference to the "standard" library was a mistake), for purposes of validating that arduino_ci has the required pieces to compile the custom library it is probably sufficient to compile against the standard library, so the above comment can be ignored.

[ianfixes]

Squashed into #183 and i've made my desired change to the NetworkLib install script

[ianfixes]

I'm embarrassed to say that I am most definitely missing something. It looks like Ethernet gets installed (or requested for install via the IDE), but I don't see it getting listed in the directory:
https://ci.appveyor.com/project/ianfixes/arduino-ci/builds/36265419

Unit testing test.cpp with g++ for mega2560... 
C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:297:in `open': No such file or directory @ dir_initialize - C:/Users/appveyor/Documents/Arduino/libraries/Ethernet (Errno::ENOENT)
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:297:in `entries'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:297:in `header_dirs'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:325:in `block in arduino_library_src_dirs'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:325:in `map'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:325:in `arduino_library_src_dirs'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:335:in `include_args'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:383:in `test_args'
	from C:/projects/arduino-ci/lib/arduino_ci/cpp_library.rb:422:in `build_for_test_with_configuration'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:238:in `block (4 levels) in perform_unit_tests'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:101:in `perform_action'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:122:in `attempt_multiline'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:237:in `block (3 levels) in perform_unit_tests'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:236:in `each'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:236:in `block (2 levels) in perform_unit_tests'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:234:in `each'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:234:in `block in perform_unit_tests'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:233:in `each'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:233:in `perform_unit_tests'
	from C:/projects/arduino-ci/exe/arduino_ci.rb:398:in `<top (required)>'
	from C:/Ruby22/lib/ruby/gems/2.2.0/bin/arduino_ci.rb:23:in `load'
	from C:/Ruby22/lib/ruby/gems/2.2.0/bin/arduino_ci.rb:23:in `<main>'
full_aux_libraries = ["Ethernet"]
after aux_libraries = ["Ethernet"]
header_dirs for C:/Users/appveyor/Documents/Arduino/libraries/Ethernet = []
from header_files: []
all items in library dir: [".", "..", "readme.txt", "USBHost"]

If the "standard" Ethernet library is used, where would that directory be?

[per1234]

It looks like Ethernet gets installed (or requested for install via the IDE)

The IDE comes with some bundled libraries:
https://github.com/arduino/Arduino/blob/a056a5fdc751b933453830434e36aeffae7512d5/build/build.xml#L266-L287

If the version of the bundled library is the latest version available from Library Manager, then the library won't be installed.

If the "standard" Ethernet library is used, where would that directory be?

libraries/Ethernet under the Arduino IDE installation folder.

[jgfoster]

One solution (and the reason it didn't fail for me before submitting the PR), is to restore the code you removed that downloads our custom Ethernet library (my comment referring to the "standard" library was wrong).

[jgfoster]

Perhaps this calls for a new issue addressing explicit inclusion of built-in (but hidden) libraries.

[per1234]

Any issues caused by the IDE's built-in libraries would be made moot if you switched to using Arduino CLI, since it doesn't have any built-in libraries.

[ianfixes]

This clears up a few things. I was looking at git clone https://github.com/arduino-libraries/Ethernet.git and thinking to myself "that does not look like the URL of a custom library, it looks like an official one..."

But with that out of the way I think I see what was happening. You have to download the Ethernet library manually into the libraries directory because arduino_ci doesn't know where to find the regular one -- i.e. this is a workaround for a bug and we should open an issue for it. The fix would likely involve some new section in the CI configuration that maps out "built-in (but hidden) libraries".

I like @per1234 's suggestion about switching to the CLI. Will that work cross-platform? If yes, I'll pull in the install script workaround for now and open a new issue to remove it if/when we switch to the CLI.

[jgfoster]

You are right, I was cloning the official library. What I though I was doing (and meant to do?) was git clone https://github.com/Arduino-CI/Ethernet. No wonder you were confused!

[per1234]

Will that work cross-platform?

Arduino CLI is available for Linux, Linux ARM, Windows, macOS. There are 32 and 64 bit versions available for the first three platforms:
https://arduino.github.io/arduino-cli/latest/installation/#latest-packages

It's a much smaller download, since it doesn't have any need for the GUI code of the IDE. The command line interface is much more powerful than the Arduino IDE's CLI.

Using the Arduino IDE's CLI for sketch compilation CI and it was frequently frustrating because when something went wrong it would start the GUI, which just means the headless Travis CI environment hangs until it times out, giving no information useful for troubleshooting. I have had a much more pleasant experience since I switched to using Arduino CLI for this purpose.

[jgfoster]

Would the CLI solve #137?

[per1234]

Would the CLI solve #137?

Yes. There's certainly nothing for it to splash!

The Arduino IDE's CLI works well enough, but it was added on as an afterthought. Arduino CLI was designed from the start with this sort of use case in mind.

[ianfixes]

Opened #206 to track the CLI, it should be straightforward if all the IDE functions have corresponding CLI functionality