Skip to content
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

Architecture review #229

Merged
merged 51 commits into from Dec 25, 2017
Merged

Conversation

user2684
Copy link
Contributor

@user2684 user2684 commented Oct 14, 2017

benchmark.xlsx
This PR is intended to review NodeManager's architecture and put some order between sensors and children, optimize the code so to use the memory in a more efficient manner and improve the overall user experience.

Architecture review (fixes #198):

  • Introduced a List helper class to facilitate dynamically storing sensors and children. The class provides an iterator and a way to access individual items;
  • Added a Child class (and a subclass for each data type) to store child-specific parameters (child_id, presentation, type, description, value_type, value and last_value). All the other variables will be common for all the children and stay within the Sensor class;
  • Added a list of Child to the Sensor class. The sensor is responsible for creating those Child and adding them to the list;
  • Added a list of Sensors to the NodeManager class, replacing the _sensors array;
  • Removed the need for the user to register a sensor. When creating a new sensor, it registers itself against NodeManager;
  • Changed Sensor constructor to only accept the instance of the NodeManager class and an optional pin
  • Removed registerSensor() and moved its logic within each sensor's class
  • Removed NodeManager's get(); the user can directly retrieve sensors and children by accessing the lists
  • Removed BATTERY_MANAGER and BATTERY_SENSOR. Added SensorBattery which can be added as any other sensor
  • Removed SIGNAL_SENSOR. Added SensorSignal which can be added as any other sensor
  • Deprecated SERVICE_MESSAGE
  • Deprecated setFloatPrecision() and setDoublePrecision(), now default to 2 and 4
  • Deprecated PERSIST, now controlled by setSaveSleepSettings()
  • Deprecated SENSOR_* defines. Now the user can create a Sensor instance by itself in the main sketch. MODULE_* defines will stay since otherwise the sketch will not compile due to missing libraries
  • Deprecated POWER_MANAGER. The user has to create a PowerManager object and then passing it to setPowerManager() available by both NodeManager and Sensor
  • Deprecated REMOTE_CONFIGURATION. Added SensorConfiguration which can be added as any other sensor.
  • Changed the remote API, now V_CUSTOM messages have to be sent to CONFIGURATION_CHILD_ID (default 200) and hence to SensorConfiguration. New format is: "child_id,function,value"
  • Re-organized the files in the sketch folder: config.h has been migrated into the main sketch so to have in a single place all the configuration directives. NodeManager.h and NodeManager.cpp renamed into NodeManagerLibrary.h and NodeManagerLibrary.cpp
  • Modules definitions are now simple defines which can be commented out
  • DEBUG define renamed NODEMANAGER_DEBUG

Code optimizations:

  • For sensors with multiple child, a single instance of Sensor is created and multiple Child are associated with it instead of having multiple instances of (the bigger) Sensor class
  • The usage of Child subclasses avoid allocating variables for each data type which are eventually never used (if _value_int was used, _value_float, value_double and _value_string were allocated anyway so consuming memory)
  • Fixed a bug which caused onProcess() functions to be compiled even if REMOTE_CONFIGURATION was off, so saving additional space
  • A List class which can be dynamically allocated replaces a static array of sensors (consuming memory when not full)
  • Analog input is split into multiple modules so to avoid consuming too much memory when the sensor is not needed
  • Optimized the usage of MyMessage, now a single global instance is used by any sensor
  • Remote configuration is off by default
  • NodeManager's code without any sensor is 2k bigger but each sensor is between 2k and 3k smaller
  • Many minor optimizations

User Experience enhancements:

  • Registering a sensor is no more needed, when declaring a sensor, this is automatically registered
  • Split between creating a sensor and configuring it (still in before())
  • Removed SENSOR_* aliases, when a module is enabled Sensor and its subclasses can be used directly
  • Merged config.h into the main sketch so to centralize the configuration in a single place
  • Removed most of NodeManager's configuration directives, not either embedded into the code or moved into dedicated sensors (e.g. remote configuration, battery, signal, etc.)
  • Simplified the configuration of each sensor, now without the need of getting the sensor back through a nasty casting. The sensor's functions can be called directly within before()
  • Simplified the code so making it simpler for a user to add a custom sensor

Bug fixes:

Examples:

NodeManager node;
SensorSHT21 sht(node);
SensorBattery battery(node);

Setting parameters to a sensor:

sht.setReportIntervalSeconds(60);

Direct access a child:

// for the first child (temperature) of the SHT21 sensor, set child_id to 5
sht.children.get(1)->child_id = 5;

Setting up a power manager for all the sensors:

NodeManager node;
PowerManager power(5,6);
void before() {
   node.setPowerManager(power);
}

@user2684 user2684 mentioned this pull request Nov 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant