Skip to content

Reorder and simplify addPropertyReal methods #343

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

Merged
merged 5 commits into from
Dec 1, 2022

Conversation

pennam
Copy link
Collaborator

@pennam pennam commented Nov 14, 2022

During the thing_id discovery work #297 i've added unnecessary methods to the public space.

The goal of this pr is to:
reorder addPropertyReal methods in ArduinoIoTCloud.cpp to make more clear who is calling who
remove unnecessary addPropertyReal methods from public space: the user should not be able to select the property container because all user properties have to be added to the _thing_property_container.

@pennam pennam requested a review from aentinger November 14, 2022 10:29
@pennam pennam changed the title Reorder ans simplify addPropertyReal methods Reorder and simplify addPropertyReal methods Nov 14, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #343 (e34a56a) into master (9c6d5d7) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #343   +/-   ##
=======================================
  Coverage   94.87%   94.87%           
=======================================
  Files          27       27           
  Lines        1113     1113           
=======================================
  Hits         1056     1056           
  Misses         57       57           
Impacted Files Coverage Δ
src/property/types/CloudString.h 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Nov 14, 2022
@pennam pennam force-pushed the reorder-addPropertyReal branch from 0ff8433 to d344c46 Compare November 14, 2022 12:49
@arduino-libraries arduino-libraries deleted a comment from github-actions bot Nov 14, 2022
@pennam
Copy link
Collaborator Author

pennam commented Nov 14, 2022

Reverted cd98ce2 and introduced d344c46 to avoid flash usage increase.

@arduino-libraries arduino-libraries deleted a comment from github-actions bot Nov 15, 2022
Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I'm a big fan of any kind of API surface reduction, so kudos 😉

@pennam
Copy link
Collaborator Author

pennam commented Nov 15, 2022

Thanks for your review @aentinger. You made me remember that after i've reverted cd98ce2 also the followings two commits are no more needed:

So i think i will cleanup this PR dropping all that is not necessary 🙇

@pennam pennam force-pushed the reorder-addPropertyReal branch 2 times, most recently from 0ff8433 to f137056 Compare November 15, 2022 18:59
@arduino-libraries arduino-libraries deleted a comment from github-actions bot Nov 15, 2022
@github-actions
Copy link

Memory usage change @ f137056

Board flash % RAM for global variables %
arduino:mbed_nano:nanorp2040connect 🔺 0 - +120 0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_nicla:nicla_vision ❔ -64 - +64 -0.0 - 0.0 0 - 0 0.0 - 0.0
arduino:mbed_portenta:envie_m7 🔺 0 - +128 0.0 - +0.02 0 - 0 0.0 - 0.0
arduino:samd:mkr1000 💚 -64 - 0 -0.02 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkrgsm1400 🔺 0 - +96 0.0 - +0.04 0 - 0 0.0 - 0.0
arduino:samd:mkrnb1500 💚 -64 - 0 -0.02 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkrwan1300 💚 -120 - 0 -0.05 - 0.0 0 - 0 0.0 - 0.0
arduino:samd:mkrwifi1010 🔺 0 - +104 0.0 - +0.04 0 - 0 0.0 - 0.0
arduino:samd:nano_33_iot 🔺 0 - +104 0.0 - +0.04 0 - 0 0.0 - 0.0
esp32:esp32:esp32 ❔ -152 - +60 -0.01 - 0.0 0 - 0 0.0 - 0.0
esp8266:esp8266:huzzah 💚 -112 - -32 -0.01 - -0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/ArduinoIoTCloud-Advanced
flash
% examples/ArduinoIoTCloud-Advanced
RAM for global variables
% examples/ArduinoIoTCloud-Basic
flash
% examples/ArduinoIoTCloud-Basic
RAM for global variables
% examples/utility/ArduinoIoTCloud_Travis_CI
flash
% examples/utility/ArduinoIoTCloud_Travis_CI
RAM for global variables
% examples/utility/Provisioning
flash
% examples/utility/Provisioning
RAM for global variables
% examples/utility/SelfProvisioning
flash
% examples/utility/SelfProvisioning
RAM for global variables
%
arduino:mbed_nano:nanorp2040connect 28 0.0 0 0.0 52 0.0 0 0.0 120 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:mbed_nicla:nicla_vision -64 -0.0 0 0.0 0 0.0 0 0.0 64 0.0 0 0.0 0 0.0 0 0.0
arduino:mbed_portenta:envie_m7 0 0.0 0 0.0 0 0.0 0 0.0 128 0.02 0 0.0 0 0.0 0 0.0
arduino:samd:mkr1000 -24 -0.01 0 0.0 -64 -0.02 0 0.0 -8 -0.0 0 0.0 0 0.0 0 0.0
arduino:samd:mkrgsm1400 0 0.0 0 0.0 32 0.01 0 0.0 96 0.04 0 0.0 0 0.0 0 0.0
arduino:samd:mkrnb1500 -24 -0.01 0 0.0 -64 -0.02 0 0.0 -8 -0.0 0 0.0 0 0.0 0 0.0
arduino:samd:mkrwan1300 0 0.0 0 0.0 0 0.0 0 0.0 -120 -0.05 0 0.0
arduino:samd:mkrwifi1010 16 0.01 0 0.0 40 0.02 0 0.0 104 0.04 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
arduino:samd:nano_33_iot 16 0.01 0 0.0 40 0.02 0 0.0 104 0.04 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
esp32:esp32:esp32 -108 -0.01 0 0.0 -152 -0.01 0 0.0 60 0.0 0 0.0
esp8266:esp8266:huzzah -64 -0.01 0 0.0 -112 -0.01 0 0.0 -32 -0.0 0 0.0
Click for full report CSV
Board,examples/ArduinoIoTCloud-Advanced<br>flash,%,examples/ArduinoIoTCloud-Advanced<br>RAM for global variables,%,examples/ArduinoIoTCloud-Basic<br>flash,%,examples/ArduinoIoTCloud-Basic<br>RAM for global variables,%,examples/utility/ArduinoIoTCloud_Travis_CI<br>flash,%,examples/utility/ArduinoIoTCloud_Travis_CI<br>RAM for global variables,%,examples/utility/Provisioning<br>flash,%,examples/utility/Provisioning<br>RAM for global variables,%,examples/utility/SelfProvisioning<br>flash,%,examples/utility/SelfProvisioning<br>RAM for global variables,%
arduino:mbed_nano:nanorp2040connect,28,0.0,0,0.0,52,0.0,0,0.0,120,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:mbed_nicla:nicla_vision,-64,-0.0,0,0.0,0,0.0,0,0.0,64,0.0,0,0.0,0,0.0,0,0.0,,,,
arduino:mbed_portenta:envie_m7,0,0.0,0,0.0,0,0.0,0,0.0,128,0.02,0,0.0,0,0.0,0,0.0,,,,
arduino:samd:mkr1000,-24,-0.01,0,0.0,-64,-0.02,0,0.0,-8,-0.0,0,0.0,0,0.0,0,0.0,,,,
arduino:samd:mkrgsm1400,0,0.0,0,0.0,32,0.01,0,0.0,96,0.04,0,0.0,0,0.0,0,0.0,,,,
arduino:samd:mkrnb1500,-24,-0.01,0,0.0,-64,-0.02,0,0.0,-8,-0.0,0,0.0,0,0.0,0,0.0,,,,
arduino:samd:mkrwan1300,0,0.0,0,0.0,0,0.0,0,0.0,-120,-0.05,0,0.0,,,,,,,,
arduino:samd:mkrwifi1010,16,0.01,0,0.0,40,0.02,0,0.0,104,0.04,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
arduino:samd:nano_33_iot,16,0.01,0,0.0,40,0.02,0,0.0,104,0.04,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0
esp32:esp32:esp32,-108,-0.01,0,0.0,-152,-0.01,0,0.0,60,0.0,0,0.0,,,,,,,,
esp8266:esp8266:huzzah,-64,-0.01,0,0.0,-112,-0.01,0,0.0,-32,-0.0,0,0.0,,,,,,,,

@aentinger
Copy link
Contributor

Glad that looking over your PR was helpful in some way =)

@arduino-libraries arduino-libraries deleted a comment from github-actions bot Nov 17, 2022
@pennam pennam merged commit 9828845 into arduino-libraries:master Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants