Skip to content

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