Add new examples #55

aentinger · 2019-04-10T18:04:06Z

No description provided.

… into add-new-examples

…noIoTCloud into add-new-examples

aentinger

@umbynos This example only compiles for MKR WIFI 1010/MKR 1000. Should MKR GSM 1400 also be supported?

examples/MKRWIFI1010_Cloud_Blink/MKRWIFI1010_Cloud_Blink.ino

Co-Authored-By: ubidefeo <[email protected]>

ubidefeo · 2019-04-10T18:14:27Z

Because of the different init process for GSM and its required parameters we should have a separate example.
I've tried to tackle a single example encompassing all supported boards (and forward-thinking) but the excess of #ifdef made me refrain from it :)
@umbynos any comments?

examples/MKRWIFI1010_Cloud_Blink/MKRWIFI1010_Cloud_Blink.ino

Co-Authored-By: lxrobotics <[email protected]>

…ed boards

… warning - otherwise travis won't build

aentinger · 2019-04-11T04:47:15Z

@ubidefeo @umbynos I've patched everything up so the file compiles also with travis for all boards. Two things:

In order to compile for all boards (and especially to cover undefined secrets) we need quite a bit of #ifdef magic - we could extract that into another file (e.g. arduino_travis_secrets.h).
Since the board works for both GSM and WIFI enabled boards I'd propose to rename the example simply to Cloud_Blink.

I'd love to hear your opinion on this 😃

umbynos · 2019-04-11T08:59:06Z

This sketch will be the one used in the getting started procedure. I've started a rework in order to use the new connection flow. So I think having one sketch for 1000, 1010 and GSM is a good thing, but maybe we should hear an opinion from @AlbyIanna.
@ubidefeo Originally there were different sketches for different boards. One for 1010, one for GSM and so on.
@lxrobotics Cloud_Blink sounds good to me.

…led boards (MKR1010 and MKR1000)

TravisBuddy · 2019-04-12T15:27:08Z

Travis tests have failed

Hey @lxrobotics,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

3rd Build

View build log

buildExampleSketch MKRWIFI1010_Cloud_Blink

Picked up JAVA_TOOL_OPTIONS: 
Loading configuration...
Initializing packages...
Preparing boards...
Verifying...
java.io.IOException: No valid code files found
	at processing.app.Sketch.listSketchFiles(Sketch.java:117)
	at processing.app.Sketch.<init>(Sketch.java:54)
	at processing.app.Base.<init>(Base.java:412)
	at processing.app.Base.main(Base.java:151)

TravisBuddy Request Identifier: 6926dd90-5d37-11e9-8db0-bbda51d68b34

TravisBuddy · 2019-04-12T15:43:10Z

Hey @lxrobotics,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: aaf45890-5d39-11e9-8db0-bbda51d68b34

umbynos · 2019-04-12T16:02:40Z

Sorry about the mess I did just above. I'm still a noob using GitHub 😅
I split MKRWIFI1010_Cloud_Blink in two examples:

WiFi_Cloud_Blink which is compatible with MKR1010 and MKR1000
GSM_Cloud_Blink which is compatible with MKRGSM1400
I did this beacuse on create all the secrets are parsed and it's not possible to hide them. So having a single sketch will generate all the secrets: some for WiFi boards and some for GSM boards.
Although I'm still having some problems with the GSM sketch, I don't know why the serial monitor remains empty.

aentinger · 2019-04-15T04:49:18Z

Good Morning @umbynos,

Don't worry about the mess, it can always be cleaned up by applying git rebase 😉 . I'll send you a confluence article which I wrote on this topic via slack.

Regardless of this I currently see two problems with this PR:

Since GSM_Cloud_Blink.ino and WiFi_Cloud_Blink.ino contain platform specific code for the MKR GSM 1400 and the MKR WIFI 1010 they can not be successfully run by our travis build script, due to the fact that the build script build each example against all platforms. @per1234 Do you have an idea how to modify the travis build script so that GSM_Cloud_Blink.ino will be only build for MKR GSM 1400 and WiFi_Cloud_Blink.ino will only be build for MKR 1000 and MKR WIFI 1010?
You are mentioning that you have problems with the serial monitor for the GSM sketch. This sounds to me like this PR is not complete yet. You can contact me on slack and maybe we can sort those issues out together in order to get this PR to completion. For future PRs: If at the time of opening a PR you are aware that there is still work to be done, please use the feature of creating a draft pull request ( https://github.blog/2019-02-14-introducing-draft-pull-requests/ ). Doing so ensures that people taking a look at your PR know it's work in progress and they can respond accordingly 😉

umbynos · 2019-04-15T07:35:26Z

Thank you! I'm going to read the article you sent me right now.

Now I understand the problem with travis errors, but I don't know how to solve them.
I did't know about draft pull request, but in the future I will certainly be using them!
Thank you for your advises.

per1234 · 2019-04-15T09:17:28Z

@per1234 Do you have an idea how to modify the travis build script so that GSM_Cloud_Blink.ino will be only build for MKR GSM 1400 and WiFi_Cloud_Blink.ino will only be build for MKR 1000 and MKR WIFI 1010?

I came up with two methods by which this could be accomplished:

Define a separate `script` phase for each job

https://github.com/per1234/ArduinoIoTCloud/commit/7a2eacfb5666641ed4b185c3f6fba20f121fa9af
build:
https://travis-ci.org/per1234/ArduinoIoTCloud/builds/520194410

Cons:

Some duplication of the script phase configuration between each job

Use conditional logic

https://github.com/per1234/ArduinoIoTCloud/commit/0976b58af4c8fea2ecf1f5a4ea68661848fc249b
build:
https://travis-ci.org/per1234/ArduinoIoTCloud/builds/520184751

Cons:

The conditional code makes .travis.yml and the job logs a bit "messy".

aentinger · 2019-04-15T09:32:40Z

Thank you very much for your input 👍

In my opinion the first version ( https://github.com/per1234/ArduinoIoTCloud/commit/0976b58af4c8fea2ecf1f5a4ea68661848fc249b ) should be implemented, since it is much simpler to read and understand (by humans). If the downside is that the CI server has a bit more work I'd say that's a small sacrifice for the increased readability (and therefore reduced probability of bug introduction).

per1234 · 2019-04-15T10:19:08Z

In my opinion the first version ( per1234@0976b58 )

Just to avoid any confusion, I should point out that per1234@0976b58 is the commit for the second version.

I don't believe that the per-job script definitions will result in any more work for the CI server. My concern with the duplication is that it makes maintenance a bit more work. For example, if a new example sketch is added to this library, it would then need to added to three separate locations in .travis.yml. I don't think that's really a significant problem though, and it's easily balanced out by the benefits of improved readability.

If you are concerned with efficiency of the CI build, a before_install phase could be defined for each job. This would allow only installing the libraries needed for that specific job. For example, there is no need to install the WiFi101 or MKRGSM libraries for the BOARD="arduino:samd:mkrwifi1010" job.

.travis.yml

Co-Authored-By: lxrobotics <[email protected]>

TravisBuddy · 2019-04-15T13:10:34Z

Travis tests have failed

Hey @lxrobotics,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: d7f65080-5f7f-11e9-8107-31db5b396b41

…obal timestamp. (#55) This is necessary because right now we are relying on the RTC within the SAMD MCU which is instantiated (RTCZero) within the ArduinoIoTCloud library and reference within ArduinoCloudThing via extern devlaration. Due to the extern binding this is a very brittle dependency which can be easily destroyed, it is therefore better to explicitly register a function which provides the time (this can be serviced by the TimeService class available in ArduinoIoTCloud

umbynos added 12 commits March 25, 2019 18:08

add new CloudBlink example

3af2e77

remove some comments

032a024

Merge branch 'master' of github.com:arduino-libraries/ArduinoIoTCloud…

2f10025

… into add-new-examples

Merge branch 'master' of github.com:arduino-libraries/ArduinoIoTCloud…

5f7ef98

… into add-new-examples

fix errors and improvements

0095a53

move example sketch in proper folder

4e02f2c

add new CloudBlink example

f0cfd21

remove some comments

0c3369a

fix errors and improvements

c51fccc

move example sketch in proper folder

70c9324

Merge branch 'add-new-examples' of github.com:arduino-libraries/Ardui…

e156a2c

…noIoTCloud into add-new-examples

apply code formatting according to examples_formatter.conf

2112403

aentinger commented Apr 10, 2019

View reviewed changes

examples/MKRWIFI1010_Cloud_Blink/MKRWIFI1010_Cloud_Blink.ino Outdated Show resolved Hide resolved

aentinger force-pushed the master branch from d372651 to 0fb6c6f Compare April 10, 2019 18:08

Update examples/MKRWIFI1010_Cloud_Blink/MKRWIFI1010_Cloud_Blink.ino

a4388b4

Co-Authored-By: ubidefeo <[email protected]>

per1234 requested changes Apr 11, 2019

View reviewed changes

examples/MKRWIFI1010_Cloud_Blink/MKRWIFI1010_Cloud_Blink.ino Outdated Show resolved Hide resolved

per1234 and others added 4 commits April 11, 2019 06:21

Update examples/MKRWIFI1010_Cloud_Blink/MKRWIFI1010_Cloud_Blink.ino

3f3a848

Co-Authored-By: lxrobotics <[email protected]>

Build 'MKRWIFI1010_Cloud_Blink.ino' with Travis

5eb39ab

MKRWIFI1010_Cloud_Blink is now compilable for both WIFI and GSM enabl…

ec30765

…ed boards

Defining arduino secrets blank in case they are undefined and issue a…

318fb5f

… warning - otherwise travis won't build

umbynos closed this Apr 11, 2019

umbynos reopened this Apr 11, 2019

split Cloud_Blink sketch, one for GSM and the other one for WiFi enab…

c029e46

…led boards (MKR1010 and MKR1000)

Update .travis.yml

9197919

per1234 requested changes Apr 15, 2019

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

.travis.yml Outdated Show resolved Hide resolved

per1234 and others added 3 commits April 15, 2019 12:43

Update .travis.yml

3c087da

Co-Authored-By: lxrobotics <[email protected]>

Update .travis.yml

833522f

Co-Authored-By: lxrobotics <[email protected]>

fix a problem with CloudSerial not being initialized

bc39f7f

add code fortmatting for GSM_Cloud_Blink

fc08f1b

aentinger merged commit 34f35cf into master Apr 15, 2019

aentinger deleted the add-new-examples branch April 15, 2019 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new examples #55

Add new examples #55

aentinger commented Apr 10, 2019

aentinger left a comment

ubidefeo commented Apr 10, 2019 •

edited

Loading

aentinger commented Apr 11, 2019 •

edited

Loading

umbynos commented Apr 11, 2019

TravisBuddy commented Apr 12, 2019

TravisBuddy commented Apr 12, 2019

umbynos commented Apr 12, 2019

aentinger commented Apr 15, 2019

umbynos commented Apr 15, 2019

per1234 commented Apr 15, 2019

aentinger commented Apr 15, 2019

per1234 commented Apr 15, 2019

TravisBuddy commented Apr 15, 2019

Add new examples #55

Add new examples #55

Conversation

aentinger commented Apr 10, 2019

aentinger left a comment

Choose a reason for hiding this comment

ubidefeo commented Apr 10, 2019 • edited Loading

aentinger commented Apr 11, 2019 • edited Loading

umbynos commented Apr 11, 2019

TravisBuddy commented Apr 12, 2019

Travis tests have failed

3rd Build

TravisBuddy Request Identifier: 6926dd90-5d37-11e9-8db0-bbda51d68b34

TravisBuddy commented Apr 12, 2019

TravisBuddy Request Identifier: aaf45890-5d39-11e9-8db0-bbda51d68b34

umbynos commented Apr 12, 2019

aentinger commented Apr 15, 2019

umbynos commented Apr 15, 2019

per1234 commented Apr 15, 2019

Define a separate script phase for each job

Use conditional logic

aentinger commented Apr 15, 2019

per1234 commented Apr 15, 2019

TravisBuddy commented Apr 15, 2019

Travis tests have failed

TravisBuddy Request Identifier: d7f65080-5f7f-11e9-8107-31db5b396b41

ubidefeo commented Apr 10, 2019 •

edited

Loading

aentinger commented Apr 11, 2019 •

edited

Loading

Define a separate `script` phase for each job