For sensors creating multiple child IDs allow invoking their functions just once

[user2684]

Issue described here: #176

[SergioRius]

If you see this as an issue, I would understand the problem as an architectural issue. The structure clearly started as something simple, node-sensor. But now we must do more complex things.
Well I think the classes should be modified to allow better hierarchical organisation and iteration.
NodeManager is the root container. Should include a collection of "Sensors" what already does. But it mixes sensors and child. Collection nodes should be accessed like this:

nodeManager.get(sensor).get(child)

At some point, you would also want to access and modify base class elements, like settings. Then you will need a differentiation like this:

nodeManager.Settings.LoopDelay = value

And then:

nodeManager.Sensors.Add(SENSOR_EXAMPLE, myOption)
nodeManager.Sensors(x).setType(V_ONETYPE)
nodemanager.Sensors(x).Child(y).setSamples(5)

And you also can have defaults and/or apply properties to all child or first child. This makes it coherent and expandable.

nodemanager.Sensors(x).Child.First.setSamples(5)
nodemanager.Sensors(x).Child.All.setSamples(5)
nodemanager.Sensors(x).setSamples(5) //All could be default
ForEach s in nodeManager.Sensors {...

[user2684]

You are 100% right, all of this is due to a mix of sensors and children which is causing all of this confusion. I think the approach you have proposed is the best possible, with an additional contained in between. I'll try to draft something as v1.7 development will start so we can check together if the direction is the right one.
Thanks again for sharing this thought!

[betonmoewe]

Maybe it would be a good idea, to have one instance of one specific sensor class with all the variables and parameters wich are global for all the different measurements (childs) of this sensor. And there is an array of pointers/IDs of all the child instances (for each measurement) of this sensor. The setter methods are either looping through this array or are called for only some of the childs.
I think, this could be a way without changing too much code ...
Best regards
Betonmoewe

[user2684]

Hi guys, thanks for the feedback so far. This issue is number 1 priority for v1.7 and I think must be addressed before adding other sensors or fixing other issues before would have an overall impact. The thing is we need to 1) define a new hierarchy 2) redefine the terminology.

Up to v1.6 we have an array of pointers to instances of Sensor and subclasses. When registering a sensor providing more than one measure (e.g. temperature and humidity), registerSensor() creates two instances of the same class and registers two child IDs by adding two pointers to the array.
Requirements are:

• registerSensor() must be called once to avoid the user to take care of registering multiple measures
• each measure must be associated to a different child id since must be presented independently
• user must be able to invoke methods for each child id of a sensor
• user must be able to invoke methods which apply to all child ids of a sensor

Let's start from Sergio's idea which I think should cover all those requirements. I struggle a bit to understand the relationship between Child and Sensor. Also, that .first. and .All., is it from a specific C++ object or it is just pseudo-code? Thanks!

[SergioRius]

That was some pseudo-C# code, but it could work on cpp. First, I would try if we can use vectors for storage (there's a small library for arduino). They allow add and remove elements, but I dunno if they are feasible on low perf boards.

We must also think on allow a function callback for "sensor" action. Then we could make virtual "physical sensors" or electronics that are not directly related to a pin for actuating, like a shift register.

[user2684]

Hi guys, I spent some time thinking about this architectural thing and get lost pretty quickly :(
Let's assume we have a Sensor class containing one or more Child.
registerSensor() will create an instance of something derived from Sensor which will create in its constructor the required Child. The good thing is registerSensor() will be cleaner since objects that need to be shared among the Child will be created in Sensor and not in registerSensor(). Child will be created by passing a reference to Sensor I guess.
Then comes the time to understand if the existing functions of Sensors need to stay there or moved into Child. Theoretically almost all must go into Child but the following:

    void setPin(int value);
      void setPowerPins(int ground_pin, int vcc_pin, int wait_time = 50);
      void setAutoPowerPins(bool value);
      void powerOn();
      void powerOff();

However if we want to apply the same setting to all the Child belonging to the sensor we need to replicate ALL the other functions which will cycle through the Child and invoke the methods. This scares me since we are already almost out of storage and this will make the code much longer, complex and resources intensive but providing little value.
An alternative could be a set of static function which I don't like much.
Any good alternative?
Thanks

[SergioRius]

You must think in what elements are properties from the sensor and what are tools from the framework.
Usually, only properties should be part of the objects. As you don't want to replicate them for every object. As a start, you should pass objects to methods of NodeManager class for doing actions.

[SergioRius]

If you want, we can talk through chat on this and perhaps do some work together. Just drop me a line when you will get on to this.

[user2684]

Ok, let me try to follow your line of thinking. Despite almost all the properties can be child-specific, this is going against the need to 1) make it simple 2) do not use additional storage. So let's assume to keep to the very minimum the child-specific properties. What properties MUST be child-specific? This is the list:

• child_id
• presentation
• type
• description
• value_type

This can be the minimum set of information a Child class can have. So let's assume a Sensor will have a data structure with those properties for each Child. This may somehow solve the problem (and creating a few others which I cannot see right now) because 1) would avoid invoking the same methods on all the child (because those parameters will be attached to Sensor and not to child) 2) would save additional space since for sensors with multiple child a single instance of the Sensor class would be created and not many.
How does it look like?

[user2684]

I spent some time thinking about the idea above and doing some initial tests and I think it could work. Each Sensor will have a data structure to store a list of Child with the information reported above. No more the need then to create multiple instances of the class for each child id. There will be one single instance for the sensor containing its child id. In the Sensor's loop(), for each Child onLoop() will be executed. The challenge will be with received() when loop() of a child only needs to be executed but I think we could find the way to make it working.

This approach could also solve a bunch of other problems: since the logic in registerSensor() is no longer needed (Sensor will create any additional objects in its constructor since all the child belonging to it will be within the instance) we could even simplify the main sketch.
Something like the following would allow the user to set any parameter to the object without the need to get and cast and do all the complex stuff required right now:

// declaring the instance directly in the main sketch
SensorAnalogInput analog(A1);
analog.setReportIntervalSeconds(60);
// register with NodeManager
nodeManager.registerSensor(analog);

Of course there will be no compatibility with previous version but I can live with it.
Hope to have some sample code to share in a couple of weeks.
Thanks

[user2684]

Hi, I've done some steps ahead (PR #229) and looking for your feedback.
First of all this is not intended to work yet, just trying to understand if it might be the right way to go based on my thoughts shared in the previous message.
Have a look at SensorSHT21 which is the only one I started changing.
The Sensor class has now a list of children in it so registerSensor will register a single instance of SensorSHT21.
Sensor::loop() has also been changed to iterate across all the children.
Performance-wise, many things can still be optimized.
But how does this approach look like to you?

[user2684]

Another few improvements in PR #229. Both _sensors and _children have been now migrated into a vector-like structure as @SergioRius recommended.
Sensors' registration has been over simplified. No more the need to register a sensor, something like the following will be enough:

// create a NodeManager instance
NodeManager nodeManager;
SensorSHT21 sht(nodeManager);
/ before
void before() {
  // setup the serial port baud rate
  Serial.begin(MY_BAUD_RATE);  
  sht.setReportIntervalSeconds(60);
}

The sensor will register itself against NodeManager. No more the need to get and cast either as you can see above. Much more intuitive I hope.
After creation sensors and children can be easily access:

nodeManager.sensors.get(0).something()
nodeManager.sensors.get(0).children.get(0).something()

The values have been moved into the Child class, however I don't like at all the mess I'm doing declaring a variable for each type (int, float,double,string) and then using only one in Sensor::loop() (same with the previous versions). I'm sure this would help us saving a good amount of storage but fixing this would go way beyond my c++ knowledge.

[user2684]

Completed architecture review #198

[user2684]

Reopening

• child_id cannot be assigned at the beginning of before() (https://forum.mysensors.org/topic/6534/nodemanager/138))

[user2684]

Closing, fixed by #251