-
Notifications
You must be signed in to change notification settings - Fork 80
chore: clean examples #426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
zfields
commented
Feb 18, 2024
•
edited
Loading
edited
- Remove/replace deprecated API calls
- Fix whitespace
45bf266
to
0cf2592
Compare
Rebased to consider latest merge |
Memory usage change @ 0cf2592
Click for full report table
Click for full report CSV
|
0cf2592
to
d3a79e7
Compare
Memory usage change @ d3a79e7
Click for full report table
Click for full report CSV
|
Memory usage change @ f617718
Click for full report table
Click for full report CSV
|
2 similar comments
Memory usage change @ f617718
Click for full report table
Click for full report CSV
|
Memory usage change @ f617718
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zfields I get your point, but I would love to keep the include of the example as equal as possible to the examples generated from the cloud editor. We have two differences:
- In Make examples build on cloud web editor #412 we have moved the
#include "arduino_secrets.h"
from the .ino to thethingProperties.h
file - We kept the
#include <Arduino_ConnectionHandler.h>
in thearduino_secrets.h
file because we need to know which secrets to include.
Moreover the AIoTC_Config.h
is somehow an internal configuration file and was not really ment to be included in the sketch files.
It appears the internal configuration is already spilling out into the example. I agree that parity between the cloud generated examples and the examples in the library is of the utmost importance. I thought changing it here would also change it there. I will change back the headers to match the cloud. My only feedback, is that it is currently rather confusing to understand where variables are coming from, and how things are being put together. |
Memory usage change @ 68fa9e5
Click for full report table
Click for full report CSV
|
@pennam I've implemented your requested changes and rebased on the latest release (1.15.1). 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
- Remove/replace deprecated API calls - Minimize file inclusions - Fix whitespace
Co-authored-by: Mattia Pennasilico <[email protected]>
Memory usage change @ 8ae88fc
Click for full report table
Click for full report CSV
|
rebased on |