espressif / esp-rainmaker

ESP RainMaker Agent for firmware development
Apache License 2.0
433 stars 146 forks source link

Adding parameter to one device, adds it to other device in specific scenario (MEGH-3727) #153

Closed jacek12345 closed 1 year ago

jacek12345 commented 1 year ago

I replicated issue on multidevice example. Scenario is as follows:

  1. Creating two additional params:
    //Create my own params
    esp_rmaker_param_t *param_my_1;
    esp_rmaker_param_t *param_my_2;
    param_my_1 = esp_rmaker_param_create("MY PARAM 1", NULL, esp_rmaker_bool(false),PROP_FLAG_READ | PROP_FLAG_WRITE);
    param_my_2 = esp_rmaker_param_create("MY PARAM 2", NULL, esp_rmaker_bool(false),PROP_FLAG_READ | PROP_FLAG_WRITE);
  2. Adding both params to switch_device:

    /* Create a Switch device and add the relevant parameters to it */
    switch_device = esp_rmaker_switch_device_create("Switch", NULL, DEFAULT_SWITCH_POWER);
    esp_rmaker_device_add_cb(switch_device, write_cb, NULL);
    
    esp_rmaker_device_add_param(switch_device, param_my_1);// adding my own param 1 to switch_device
    esp_rmaker_device_add_param(switch_device, param_my_2);// adding my own param 2 to switch_device
    esp_rmaker_node_add_device(node, switch_device);
  3. adding only one (first) param to light_device

    /* Create a Light device and add the relevant parameters to it */
    light_device = esp_rmaker_lightbulb_device_create("Light", NULL, DEFAULT_LIGHT_POWER);
    esp_rmaker_device_add_cb(light_device, write_cb, NULL);
    
    esp_rmaker_device_add_param(light_device,
            esp_rmaker_brightness_param_create(ESP_RMAKER_DEF_BRIGHTNESS_NAME, DEFAULT_LIGHT_BRIGHTNESS));
    
    esp_rmaker_device_add_param(light_device,param_my_1);// adding my own param 1 only to light_device - also param 2 will add
    
    esp_rmaker_device_add_attribute(light_device, "Serial Number", "012345");
    esp_rmaker_device_add_attribute(light_device, "MAC", "xx:yy:zz:aa:bb:cc");
    
    esp_rmaker_node_add_device(node, light_device);
  4. Result: also second parameter is added to light_device. From monitor: I (4892) esp_rmaker_param: Reporting params (init): {"Switch":{"Name":"Switch","Power":true,"MY PARAM 1":false,"MY PARAM 2":false},"Light":{"Name":"Light","Power":true,"Brightness":25,"MY PARAM 1":false,"MY PARAM 2":false},"Fan":{"Name":"Fan","Power":false,"Speed":3},"Temperature Sensor":{"Name":"Temperature Sensor","Temperature":25.00000},"Time":{"TZ":"Europe/Warsaw","TZ-POSIX":"CET-1CEST,M3.5.0,M10.5.0/3"},"Schedule":{"Schedules":[]},"Scenes":{"Scenes":[]},"Local Control":{"POP":"62735eb3","Type":1}}

Additional informations:

shahpiyushv commented 1 year ago

@jacek12345 it is not allowed to add one parameter into multiple devices. We will add checks internally so that the esp_rmaker_device_add_param() API will throw an error if the given param is already part of another device.

In your case, please create a new param and then add it to the light.

jacek12345 commented 1 year ago

There is in docummentation, that Parameter name should be unique in a given device. image

jacek12345 commented 1 year ago

Ok, I understand, parameter name is not the same as parameter handle. We can't add the same parameter (using the same handle) to different devices. But I'm not sure if it shouldn't be possible with RainMaker. Especially if it generally works if we use one common parameter. It looks rather that there is problem in esp_rmaker_device_add_param()

shahpiyushv commented 1 year ago

@jacek12345 Yes, the name can repeat in other devices, but the param object itself needs to be unique. So, you will have to create a 3rd custom param (with the same name "MY PARAM 1" if you wish) and then add it to your light device.

shahpiyushv commented 1 year ago

@jacek12345 , the parameter object itself has quite some metadata associated with it, including its value, bounds, UI type, etc. Even if bounds, UI type, etc. could be same, the value itself would typically not be the same and will have to be individually modifiable and hence, require different objects.

jacek12345 commented 1 year ago

OK, thank You. Closing, but rethink it please, as it would be very convenient to have some main multidevice parameters viewable (and modifiable or not) from different device tabs. Especially, that it works if we use only one 'common' parameter (when modified in one device, it is reported also to other devices that have this parameter registered). Is it fortuitously?