More MQTT Ethernet configuration - timeout

[mobrembski]

Hi All!

I will give you a background why i've created this PR.

My device is quite standalone, and it doesn't requires MQTT broker to be connected all the time. It's great if it is connected, but if it's not connected - it should be able to still operate normally.
The problem is that current MySensors library in practice doesn't allow to run code without connection to broker. In practice, because in theory it allows but when i run my code on my device, user loop() is handled every 2 seconds because MQTT is blocking MCU while waiting for connection to be established. If it fails, it runs user loop for one time and then it retries to connect, blocking for another second.
It is not very useful to have application loop executed every 2 seconds...
My solution is very simple. Just give end user ability to change default connection timeout settings. If he doesn't supply his customized settings, then default (1s) is used - just like it is now.

[mfalkvidd]

How about setting the default value in MyConfig.h (like we do for baud rate here). That way we don't need to have the if clause here and on line 258.

[mobrembski]

As you wish, no problem! Will do in an hour

[mobrembski]

done

[mfalkvidd]

Nice work. Our CI pipeline is out of order at the moment unfortunately, probably due to changes in the Github API. People in the core team are looking at it.

In the meanwhile, here are some minor things:

1. Could you please add the new keywords to keywords.txt? If keywords aren't in keywords.txt, they will not be highlighted in the Arduino IDE. Highlighting makes the code easier to follow and helps spot spelling mistakes.
   Just add these here
   MY_MQTT_ETH_CLIENT_CONNECTION_TIMEOUT
   MY_MQTT_ETH_INIT_DELAY

2. This is a bit quirky, but to make Doxygen generate documentation, the new keywords must be defined at

   MySensors/MyConfig.h

   Line 2344 in 947f1a0

   #define MY_MQTT_CLIENT_PUBLISH_RETAIN

If keywords aren't included in Doxygen, they will not be available at https://www.mysensors.org/apidocs/index.html

Just add defines for these:
MY_MQTT_ETH_CLIENT_CONNECTION_TIMEOUT
MY_MQTT_ETH_INIT_DELAY

[mobrembski]

Nice work. Our CI pipeline is out of order at the moment unfortunately, probably due to changes in the Github API. People in the core team are looking at it.

In the meanwhile, here are some minor things:

1. Could you please add the new keywords to keywords.txt? If keywords aren't in keywords.txt, they will not be highlighted in the Arduino IDE. Highlighting makes the code easier to follow and helps spot spelling mistakes.
   Just add these here
   MY_MQTT_ETH_CLIENT_CONNECTION_TIMEOUT
   MY_MQTT_ETH_INIT_DELAY
2. This is a bit quirky, but to make Doxygen generate documentation, the new keywords must be defined at

   MySensors/MyConfig.h

   Line 2344 in 947f1a0

   #define MY_MQTT_CLIENT_PUBLISH_RETAIN

If keywords aren't included in Doxygen, they will not be available at https://www.mysensors.org/apidocs/index.html

Just add defines for these:
MY_MQTT_ETH_CLIENT_CONNECTION_TIMEOUT
MY_MQTT_ETH_INIT_DELAY

No problem!
Done ;)

[mobrembski]

It appears that current EthernetClient implementation for Linux GW doesn't support setSocketTimeout. But it think it can be added easily, will try....

[mobrembski]

I've added implementation of setConnectionTimeout based on setsockopt SO_SNDTIMEO and SO_RCVTIMEO, please have a look.
I'm not checking return status for setSockOpt since setting SO_SNDTIMEO and SO_RCVTIMEO is not obligatory and it will result in working socket even if it fails.

[mobrembski]

Unfortunately, still ESP platforms doesnt support connection timeout, and i can't see an option to implement it...
So i need to ifdef around that

[mobrembski]

Hi all, i can see butler assertion:
A mail with a patch has been sent to you that, if applied to your PR, will make it follow the MySensors coding standards.
You can apply the patch using:
git apply restyling.patch

is there any option to get what he has produced? I can't find what's wrong in coding style

[mfalkvidd]

@mobrembski you didn't get the email?
Can you access https://ci.mysensors.org/job/MySensors/job/MySensors/job/PR-1453/lastBuild/execution/node/4/ws/MySensors/restyling.patch ?

[mobrembski]

@mfalkvidd No, i didnt ;) that's why i was asking :)
Thanks for link!

[mfalkvidd]

I'll check the log files to see if I can find anything related to the missing email.

[mfalkvidd]

I think gsm has same problem as esp.

/var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1453/workspace/MySensors/core/MyGatewayTransportMQTTClient.cpp:256:18: error: 'TinyGsmClient {aka class TinyGsmSim800::GsmClient}' has no member named 'setConnectionTimeout'; did you mean 'setTimeout'?
_MQTT_ethClient.setConnectionTimeout(MY_MQTT_ETH_CLIENT_CONNECTION_TIMEOUT);
^~~~~~~~~~~~~~~~~~~~
setTimeout
exit status 1