Separate Accessories - Causes Intermittent 'No Response'

[mriksman]

Hi,

When I use the below code to create a device with separate accessories, both will intermittently go 'No Response'. Perhaps something to do with using the same ACCESSORY_INFORMATION Service (and NAME)?

Does each Accessory need a different name set in the ACCESSORY_INFORMATION Service? It appears homekit_setup_mdns only uses the name from accessories[0]?

Do I need to specifically set .id for each Accessory, or is this handled by the homekit_accessories_init function?

void init_accessory() {
    light_service_t *light = lights;

    // not really needed. only used to identify a light service by name
    uint8_t hk_light_idx = 1;

    // create an array of homekit_service_t pointers. 
    // remember, this on the stack and will be discarded when the function ends (but is copied to heap in NEW_HOMEKIT_ACCESSORY)
    homekit_service_t* services[5];   
    // get pointer to first array location 
    homekit_service_t** s = services;

    // accessories[] is a global array of homekit_accessory_t pointers
    homekit_accessory_t** a = accessories;

    homekit_service_t* accessory_service = NULL;

    uint8_t macaddr[6];
    esp_read_mac(macaddr, ESP_MAC_WIFI_STA);
    int name_len = snprintf( NULL, 0, "esp-%02x%02x%02x", macaddr[3], macaddr[4], macaddr[5] );
    char *name_value = malloc(name_len + 1);
    snprintf( name_value, name_len + 1, "esp-%02x%02x%02x", macaddr[3], macaddr[4], macaddr[5] ); 

    while (light) {
        // if not a remote and not hidden (homekit enable)
        if (!light->is_remote && !light->is_hidden) {
            // first pass through, create the accessory service to be common for all accessories (if separate accessories)
            if (!accessory_service) {

                esp_app_desc_t app_desc;
                const esp_partition_t *running = esp_ota_get_running_partition();
                esp_ota_get_partition_description(running, &app_desc);

                // NEW_HOMEKIT_SERVICE uses calloc to save the data on the heap, then returns the address which is saved in a local pointer
                //  The NEW_HOMEKIT_ACCESSORY copies this pointer to a memory location on the heap
                accessory_service = NEW_HOMEKIT_SERVICE(ACCESSORY_INFORMATION, .characteristics=(homekit_characteristic_t*[]) {
                    NEW_HOMEKIT_CHARACTERISTIC(NAME, name_value),
                    NEW_HOMEKIT_CHARACTERISTIC(MANUFACTURER, "Riksman"),
                    NEW_HOMEKIT_CHARACTERISTIC(SERIAL_NUMBER, name_value),
                    NEW_HOMEKIT_CHARACTERISTIC(MODEL, "ESP-C3-12F"),
                    NEW_HOMEKIT_CHARACTERISTIC(FIRMWARE_REVISION, app_desc.version),
                    NEW_HOMEKIT_CHARACTERISTIC(IDENTIFY, status_led_identify),
                    NULL
                });

                // only occurs on the first accessory created. and if all lights are part of the one accessory (separate=false), 
                //  then we need to add the above to the list of services just once
                *(s++) = accessory_service;

            }

            int light_name_len = snprintf(NULL, 0, "Light %d", hk_light_idx);
            char *light_name_value = malloc(light_name_len + 1);
            snprintf(light_name_value, light_name_len + 1, "Light %d", hk_light_idx);

            homekit_characteristic_t* characteristics[light->is_dimmer ? 3 : 2]; // NAME, ON and if a dimmer BRIGHTNESS
            homekit_characteristic_t** c = characteristics;

            *(c++) = NEW_HOMEKIT_CHARACTERISTIC(NAME, light_name_value);
//            *(c++) = NEW_HOMEKIT_CHARACTERISTIC(NAME, "Light");
            *(c++) = NEW_HOMEKIT_CHARACTERISTIC(
                    ON, false,
    //                .callback=HOMEKIT_CHARACTERISTIC_CALLBACK(
    //                    lightbulb_on_callback, .context=(void*)light
    //                ),
                    .setter_ex=light_on_set, 
                    .getter_ex=light_on_get,
                    .context=(void*)light
                );
            if (light->is_dimmer) {
                *(c++) = NEW_HOMEKIT_CHARACTERISTIC(
                        BRIGHTNESS, 100,
                        .callback=HOMEKIT_CHARACTERISTIC_CALLBACK(
                            lightbulb_brightness_callback, .context=(void*)light
                        ),
                    );
            }
            *(c++) = NULL;

            // if dimmer, use LIGHTBULB Service to use BRIGHTNESS Characteristic
            // if not a dimmer, use SWITCH Service, so we can choose the icon to be a Fan or Light.
            if (light->is_dimmer) {
                *s = NEW_HOMEKIT_SERVICE(LIGHTBULB,  .characteristics=characteristics);
            }
            else {
                *s = NEW_HOMEKIT_SERVICE(SWITCH,  .characteristics=characteristics);
            }
            // save the address returned by calloc into light->service as well
            light->service = *s;
            s++;

            // if each light/service is to be a separate accessory
            if (separate_accessories) {
                // end this services array
                *(s++) = NULL;
                // create the accessory (copies the local array of homekit_service_t pointers to the heap) 
                //  and save the calloc address to the global accessories[] array
                *(a++) = NEW_HOMEKIT_ACCESSORY(.category=homekit_accessory_category_lightbulb, .services=services);

                // reuse the services array and start again with a new accessory
                s = services;
                // each new accessory needs the accessory_service
                *(s++) = accessory_service;

            }

            // only used for naming lights. not really required
            hk_light_idx++; 
        }

        // linked list, next light
        light = light->next;
    }

    // out of all the lights (light_service_t linked list), were there any that were homekit enabled? 
    //  if accessory_service exists, then the answer is 'yes'
    if (accessory_service) {
        // if it isn't separate accessories, then end the accessory definition
        if (!separate_accessories) {
            *(s++) = NULL;
            *(a++) = NEW_HOMEKIT_ACCESSORY(.category=homekit_accessory_category_lightbulb, .services=services);
        }

        // must end the array of pointers with a NULL
        *(a++) = NULL;

        // start the homekit server
        homekit_server_init(&config);
    }
    // if homekit doesn't initialise, we still need to start the mdns service
    else {
        mdns_init();   
        mdns_hostname_set(name_value);
        mdns_instance_name_set(name_value);
    }

}

[maximkulkin]

Hi

Yes, homekit_setup_mdns() uses name of the first accessory only. It is considered to be "the main" accessory and thus it's category and name is used. This is the case for accessory bridges: the bridge accessory goes first, then all other accessories. Thus, overall the advertised name of the accessory and it's category is derived from accessories[0]. esp-homekit does not support running multiple servers on the same device.

All IDs are automatically assigned if you did not assign them manually. It traverses the accessory-service-characteristic graph in order the are defined and assigns IDs sequentially unless it encounters non-zero ID in which case it will keep it and continue to assign IDs from the next ID. This is done to support controlling IDs in case some accessory/service/characteristic was removed in new firmware, but you want to keep all other accessory/service/characteristic IDs unchanged (and this is the advised way of doing it in HomeKit spec).

Regarding your code, I have found one problem. When you allocate characteristics array with size depending on whether it is dimmer or not, you're off by one: you need an extra element for the NULL pointer at the end. I also would recommend just having array of size 4, it does not hurt and it is less error prone.

[mriksman]

Regarding your code, I have found one problem. When you allocate characteristics array with size depending on whether it is dimmer or not, you're off by one: you need an extra element for the NULL pointer at the end. I also would recommend just having array of size 4, it does not hurt and it is less error prone.

Nice catch! I doubt this is what is causing my issue though...? Or maybe it is..?

Can you think of anything that would be causing my accessories that are separated to intermittently go 'No Response'?

[maximkulkin]

When you say "both accessories will go No Response", "both" means configured accessories within one physical device, right? Did you check console logs? Does it crash?

[mriksman]

Correct. Single/physical ESP controller, but with the separate_accessories flag set, so I can place them in different rooms.

I changed it to characteristics[4], but the issue remains.

I do not think they are crashing; the status LED flashes for 10 seconds after a reboot, and I haven't noticed this. I will double-check and revert back.

[mriksman]

Should I be creating a separate Accessory with category homekit_accessory_category_bridge? And then add each homekit_accessory_category_lightbulb as separate accessories after that? It does seem strange that one light has 'Bridge >' in its settings, which links back to the other light.

I want to add some Custom Characteristics to log number of restarts. But; I do not want to add it to all lights, or even just one light. Can you add Custom Characteristics to the 'Bridge' accessory? So something like this;

Accessories
  Bridge Accessory
    Accessory Information Service
      Name Characteristic
      Firmware Characteristic
      **Custom Characteristic**  
  Lightbulb Accessory (1)
    Accessory Information Service
      Name Characteristic
      Firmware Characteristic
   Lightbulb Service
      On Characteristic
      Brightness Characteristic 
  Lightbulb Accessory (2)
    Accessory Information Service
      Name Characteristic
      Firmware Characteristic
   Lightbulb Service
      On Characteristic
      Brightness Characteristic 

Normally I add the custom characteristic to the main service (I added some to a HEATER_COOLER), and it shows up in the Eve App.

Looking at your code, I don't see anything specific to Bridge Accesories. Homebridge and Espressif's HomeKit SDK both set some kind of is_bridge flag for the first Accessory..

[mriksman]

I have added the Custom Characteristic that tracks number of restarts, and last restart reason. After seeing the device go 'No Response' for a few minutes and come back, I checked the Characteristic in Eve, and found that it had not restarted. So something else is happening.

Any other thoughts?