espressif / esp-matter

Espressif's SDK for Matter
Apache License 2.0
697 stars 157 forks source link

General design of app_bridged_device (CON-961) #794

Open jonsmirl opened 10 months ago

jonsmirl commented 10 months ago

You might want to consider reworking the bridge code into using a callback model. I did not find app_bridged_device to be useful. Instead, I pulled it into my code and rewrote it. When you see code like this, it is a sure sign that callbacks are needed:

/** ZigBee Device APIs */
app_bridged_device_t *app_bridge_get_device_by_zigbee_shortaddr(uint16_t zigbee_shortaddr);

uint16_t app_bridge_get_matter_endpointid_by_zigbee_shortaddr(uint16_t zigbee_shortaddr);

uint16_t app_bridge_get_zigbee_shortaddr_by_matter_endpointid(uint16_t matter_endpointid);

/** BLE Mesh Device APIs */
app_bridged_device_t *app_bridge_get_device_by_blemesh_addr(uint16_t blemesh_addr);

uint16_t app_bridge_get_matter_endpointid_by_blemesh_addr(uint16_t blemesh_addr);

uint16_t app_bridge_get_blemesh_addr_by_matter_endpointid(uint16_t matter_endpointid);

/** ESP-NOW Device APIs */
app_bridged_device_t *app_bridge_get_device_by_espnow_macaddr(uint8_t espnow_macaddr[6]);

uint16_t app_bridge_get_matter_endpointid_by_espnow_macaddr(uint8_t espnow_macaddr[6]);

uint8_t* app_bridge_get_espnow_macaddr_by_matter_endpointid(uint16_t matter_endpointid);

The way I handled this is with private device types for bridged devices. For example, if you used Zigbee to bridge a temp sensor, the bridged device gets two device types -- temp_sensor + zigbee_temp_sensor. The saved device type to flash would be zigbee_temp_sensor so that when it gets restored you get a callback with that special device type.

Now app_bridge_get_device_by_zigbee_shortaddr() can turn into app_bridge_get_device(addr, callback). That callback sees the zigbee_temp_sensor device type can decode an opaque address struct. The callback checks the custom device type and then only decodes the address struct for device types it knows. Switching to a callback model based on custom device types allows app_bridged_device to disappear and be pushed into the core. Once you do this you don't have to store "app_bridged_device_address_t dev_addr" since you can get it from "esp_matter_bridge::device_t dev". The custom device type tells you how to cast.

The trick to making this work is adding the secondary device types onto the existing devices. So temp_sensor + zigbee_temp_sensor instead of just temp_sensor, temp_sensor + blemesh_temp_sensor instead of temp_sensor, etc. This has no impact on Google, Apple, or Amazon... they just ignore the secondary device types. I am using vendor device types for this. This also has the side effect of letting me know which devices are bridged in my app allowing me to enable my bridged device control UI. With app_bridge_create_bridged_device() you use the custom device type.

I would find a generalized model more useful since I've created two bridged device types different than the three you made.

InfiniteYuan commented 10 months ago

Sounds good

Can you provide some code prototype? We will consider making modifications in future development.

BTW, this currently codes belongs to the category of example code and only provides a reference for application project. There may be more good ways to achieve this.