monstrenyatko / ArduinoMqtt

MQTT client for Arduino
MIT License
72 stars 12 forks source link

Make NET_MAX_TM_MS and NET_MIN_TM_MS configurable #12

Closed tarzan115 closed 7 years ago

tarzan115 commented 7 years ago

hi @monstrenyatko

I have a problem. I checked in your code, the value NET_MIN_TM_MS min is 100L. that mean loop function will delay 100 ms. can I set NET_MIN_TM_MS smaller to make loop function faster or another way to make loop faster by just run when having data or something like that.

thank you so much. 😄

monstrenyatko commented 7 years ago

Hi @tarzan115 The loop delay usually is caused by the value provided to MqttClient::yield function. If you do not specify the value the default one is 1000L.

The NET_MIN_TM_MS is used to restrict some minimum amount of time for recvPacket or sendPacket functions when the global timer might be not enough.

If you need to minimize the delay caused by MqttClient::yield you need to specify a smaller parameter value. If you use a value smaller than 100, Yes you need to set the NET_MIN_TM_MS smaller as well but this delay should be big enough to receive the entire packet.

Please keep in mind that the delay provided to MqttClient::yield must be big enough to receive the packet, process it and send acknowledge back to the server if required (see QoS1-2 subscriptions).

Unfortunately, we don't have a mechanism to modify the NET_MAX_TM_MS or NET_MIN_TM_MS externally right now...I'll add additional constructor parameters at the end of the month when I get access to the hardware to verify.

Please try to modify the library values directly and let me know about the results.

tarzan115 commented 7 years ago

I modified NET_MIN_TM_MS to 10L and it works well, and when reduce to 1L is making disconnect to Broker in sometimes. The idea is just only when having an event from Broker, the library takes a time to receive data.

monstrenyatko commented 7 years ago

@tarzan115 I got your point. Let's add additional constructor parameters to be able to modify the NET_MAX_TM_MS and NET_MIN_TM_MS values as the quick solution. In parallel, I'll think about optimization of the MqttClient::yield call to work more reliable with small values of the NET_MIN_TM_MS.

tarzan115 commented 7 years ago

okay, thank you so much 😄

tarzan115 commented 7 years ago

I just changed MqttClient::yield function from

void yield(unsigned long timeoutMs = 1000L) {
        Timer timer(mSystem, timeoutMs);
        MQTT_LOG_PRINTFLN("Yield for %lu ms", timer.leftMs());
        do {
            ReadPacketResult result;
            cycle(result, timer);
        } while (isConnected() && !timer.expired());
    }

to

void yield(unsigned long timeoutMs = 1000L) {
        Timer timer(mSystem, timeoutMs);
        MQTT_LOG_PRINTFLN("Yield for %lu ms", timer.leftMs());
        if(isConnected())
        {
            ReadPacketResult result;
            cycle(result, timer);
        }
    }

that means we don't need while() in yield() because I think Arduino have loop() function and it runs again again and again so we don't need while() anymore. maybe I'm wrong 😄 . Can you please check that. As I can see in do while() function, you declare result variable for each time in while() that mean in whole do while() we just take only one result. so I think just replace if() instead while().

Please check that. thank you so much for the great library 😃

monstrenyatko commented 7 years ago

The idea of the MqttClient::yield is to provide the execution context to the MqttClient for some period of time to process incoming messages from the broker. Without while it will exit immediately. You can try to call MqttClient::yield with 0 If you need to avoid internal while. Using of the if is not good because I would like to have at least one call to the cycle method.

tarzan115 commented 7 years ago

I checked again and find out not in MqttClient::yield take the time, function recvPacket take it. and when finished cycle method, timer.expired() getting true and while just run only one time.

monstrenyatko commented 7 years ago

What is the value do you use for MqttClient::yield?

tarzan115 commented 7 years ago

I set MqttClient::yield is 1

monstrenyatko commented 7 years ago

For yield method the 1 means 1ms. It explains why timer expires after the first call to cycle. If you set bigger value (default value is 1000) the while will loop a few times, depends on the time required for single cycle execution.

In your case, the NET_MIN_TM_MS will be the main parameter to control yield execution time.

tarzan115 commented 7 years ago

Okay, thank you so much 😃