Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NET_MAX_TM_MS and NET_MIN_TM_MS configurable #12

Closed
tarzan115 opened this issue Aug 14, 2017 · 11 comments
Closed

Make NET_MAX_TM_MS and NET_MIN_TM_MS configurable #12

tarzan115 opened this issue Aug 14, 2017 · 11 comments

Comments

@tarzan115
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

@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
Copy link
Author

okay, thank you so much 😄

@tarzan115
Copy link
Author

tarzan115 commented Aug 16, 2017

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 😃

@tarzan115 tarzan115 reopened this Aug 16, 2017
@monstrenyatko
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@tarzan115
Copy link
Author

I set MqttClient::yield is 1

@monstrenyatko
Copy link
Owner

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
Copy link
Author

Okay, thank you so much 😃

@monstrenyatko monstrenyatko reopened this Aug 18, 2017
@monstrenyatko monstrenyatko changed the title can I reduce NET_MIN_TM_MS smaller? Make NET_MAX_TM_MS and NET_MIN_TM_MS configurable Aug 18, 2017
This was referenced Aug 24, 2017
monstrenyatko added a commit that referenced this issue Aug 31, 2017
Added optional parameters to the constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants