search

openvehicles / Open-Vehicle-Monitoring-System-3

Open Vehicle Monitoring System - Version 3
http:///www.openvehicles.com/
Other
609 stars 231 forks source link

Mqtt Command Throttling #1056

Open Jamidd opened 3 months ago

Jamidd commented 3 months ago

This PR aims to prevent excessive command spamming via MQTT, which could potentially cause the device to crash.

dexterbg commented 2 months ago

Jaime, I've got some issues with this.

  1. The variable name is confusing.
  2. The implementation will block all incoming messages from being processed.
  3. Incoming commands will not be throttled but dropped.

On 1: please generally take care naming variables according to their function: m_accept_command implies false/0 will block commands. Instead it's vice versa. A better name would be m_block_command, or even better m_command_block_ticker to also reflect it's a timer variable.

On 2: to properly throttle commands, move the throttling into OvmsServerV3::IncomingMsg(), and only apply it to the command/… topic scheme.

On 3: dropping commands is bad, as MQTT clients normally cannot check for responses and issue resubmissions easily. Instead of dropping the command, queue it. Add the queue check & delayed execution to the per second ticker handler.

While you're at it, making the delay and queue size configurable is also a good idea.

Regards, Michael