Not all lights are changing when sending lots of requests

[jespertheend]

Hi, I have many different lights that each have a different group (12 total). When I send a lot of requests to change the brightness, color, on/off state etc. It will send all the commands in the order that I sent the requests. But at some point it stops sending commands and the last few lights in the chain don't change. Perhaps I'm not understanding the settings correctly, but when I lower the value of packet_repeats, all update commands get sent faster, but accuracy suffers and once in a while a light doesn't update correctly. I feel like there is some sort of command buffer that has reached its limit. Is there a setting somewhere to increase this limit? I'd like to still send all commands while also maintaining accuracy.

[sidoh]

Hello!

Your hunch is probably correct. There's a packet queue that can fill up. When it does (the max size is 20), old packets are discarded to make way for new ones.

You can view queue stats under the /about route (json path: .queue_stats), e.g. --

{"firmware":"milight-hub","version":"1.10.4","ip_address":"10.133.8.158","reset_reason":"Software/System restart","variant":"d1_mini","free_heap":17456,"arduino_version":"2_4_2","queue_stats":{"length":0,"dropped_packets":0}}

queue_stats.dropped_packets being >0 will tell you for sure if this is the problem.

The size of the queue is currently not configurable at runtime, although you could compile the firmware yourself with MILIGHT_MAX_QUEUED_PACKETS set to whatever you want. Keep in mind that you probably only have ~5KiB of memory to play with before you start running into issues, though.

[jespertheend]

Thanks! Setting MILIGHT_MAX_QUEUED_PACKETS to 100 seems to have done the trick. free_heap is usually around 15000 for me, even when sending a lot of requests. Either way, everything seems to be running fine. It's hard to say though because every time I refresh /about all requests get stalled for a few seconds and free_heap doesn't actually seem to decrease.

Is the memory limit the reason why MILIGHT_MAX_QUEUED_PACKETS is not a configurable setting? It would be nice to not have to modify this value every time there's an update. I might be able to do a PR if it's not too difficult.

[sidoh]

No good reason this isn't configurable; just seemed unlikely folks would want to change it if a good default was chosen. Happy to collaborate on a PR. Let me know if you want some pointers.

The buffer grows/shrinks dynamically right now (which I suspect is causing memory leaks, but haven't verified. so this may change), meaning there is currently not much cost to it increasing arbitrarily.

[jespertheend]

Alright cool, I'll see if I can get something working.

[jespertheend]

Hmm ok, this is how far I managed to get: https://github.com/jespertheend/esp8266_milight_hub/commit/88b7c4d7a67d83c93051601279fb923d1ce44b7a. But C++ is not my most fluent language and I'm stuck trying to pass the Settings instance along to PacketQueue.cpp. I added Settings& settings as argument to PacketQueue but I'm getting this error: no matching function for call to 'PacketQueue::PacketQueue(). So I'm guessing that's because PacketQueue get's constructed here and I'm not really sure how to pass the Settings instance as an argument in a header file.

Should I add PacketQueue as pointer in main.cpp like the rest of the classes? Or should I create a new method in PacketQueue that takes Settings as argument, so I can pass the settings instance along after constructing PacketQueue? Or perhaps there's a better way to do this that I'm not aware of?

Other than that I don't think there's anything else that needs to be done.