espressif / esp-now

A connectionless Wi-Fi communication protocol
Apache License 2.0
477 stars 90 forks source link

Control attribute is mostly meaningless (AEGHB-306) #82

Closed n3b0j5a closed 5 months ago

n3b0j5a commented 12 months ago

I tried using espnow control library to exchange data simple app (like switch and bulb) but i don't find attributes very useful in current form or i dont understand how it is intended to work.

espnow_ctrl_initiator_send(ESPNOW_ATTRIBUTE_KEY_1, ESPNOW_ATTRIBUTE_POWER, status);

This is from coin cell demo example. Now how to send switch battery status to the receiver? What if we have 2 switches and 2 light bulbs in the network? Should we use device attribute as an UID? Imho, this should be refined/reworked, but overall idea is great.

lhespress commented 12 months ago

@n3b0j5a Just as https://github.com/espressif/esp-now/blob/master/src/control/include/espnow_ctrl.h#L26 description, the device attribute is used as a simple UID, it only provide the most frequently used attribute. for battery, you can reference the define as follows:

ESPNOW_ATTRIBUTE_BATTERY_BASE    = 0x0300,
ESPNOW_ATTRIBUTE_STATUS_LOW_BATTERY          = 0x0301,
ESPNOW_ATTRIBUTE_BATTERY_LEVEL          = 0x0302,
ESPNOW_ATTRIBUTE_CHARGING_STATE          = 0x0303,
lhespress commented 12 months ago

@n3b0j5a For 2 switches and 2 light bulbs, the control based on bind list which based on MAC. The code links:

https://github.com/espressif/esp-now/blob/master/src/control/src/espnow_ctrl.c#L205 https://github.com/espressif/esp-now/blob/master/src/control/src/espnow_ctrl.c#L281

n3b0j5a commented 11 months ago

@lhespress Still i don't like the solution, doesn't seem natural. In this case, i need to bind my switch twice (once for switch, and second time for battery).

I will make another PR to add battery and some other useful functionality, that looks handy.

n3b0j5a commented 11 months ago

@lhespress We are developing new device which will use esp-now, so i will probably contribute often, when I need some feature or I find bug. Overall interesting project. Here you go: https://github.com/espressif/esp-now/pull/83

lhespress commented 11 months ago

@n3b0j5a Could you share some information about your project?

n3b0j5a commented 11 months ago

Its under NDA for now, but we are using ESP32 for quite some time. Should be high volume consumer product with IoT access. Should be in field testing in September.

lhespress commented 11 months ago

@n3b0j5a Thanks for your sharing, If you have any problems, you can bring them up and I'll do it ASAP.

n3b0j5a commented 10 months ago

I will reopen this issue. At the moment, i had to drop using control feature of espnow protocol in favor of having custom one. With current implementation I have to bind for each attribute data that I'm sending, and if the communication is bidirectional, number of required binds is even greater. Bind that is based solely on MAC address would be much better, or MAC address + capabilities, but one bind per device and it should be bidirectional.