Feature/addsetters

[nzfarmer1]

Please review. thank you.

[owagner]

Looks fine, two comments:

-- The python paho module will automatically reconnect if the connection to the MQTT broker gets lost, so the manual reconnection appears to be redundant. This is documented, albeit very cursory, in the description for loop_start()

-- I'd remove the "ctrl^c" print -- it's pretty obvious that you can exit the module this way, and it would be the only output of the module not being done via the logging framework

[nzfarmer1]

Are you sure it will reconnect when using just loop - same thread? We've removed loop_start

Please consider the environment before printing this email.

Any information contained within this email is for the use of the recipient only and is sent in confidence. The information may not be copied, distributed or forwarded to any other parties. This information may not be used by any other person or organization. If you have received this in error, please notify us immediately by return mail and return the message with your notification.

If you would like to stop receiving correspondence from this email address, please confirm by return mail.

On 2/08/2015, at 9:05 pm, Oliver Wagner notifications@github.com wrote:

Looks fine, two comments:

-- The python paho module will automatically reconnect if the connection to the MQTT broker gets lost, so the manual reconnection appears to be redundant. This is documented, albeit very cursory, in the description for loop_start()

-- I'd remove the "ctrl^c" print -- it's pretty obvious that you can exit the module this way, and it would be the only output of the module not being done via the logging framework

— Reply to this email directly or view it on GitHub.

[owagner]

Ok, noted, but why did you switch from loop_start() to a manual loop then?

[nzfarmer1]

To ensure subscribe events would be triggered. It was a very tight Modbus loop with only 1 second of sleep, so calling mqtt.loop() ensures mqtt house keeping will be done.

Ok, noted, but why did you switch from loop_start() to a manual loop then?

— Reply to this email directly or view it on GitHub.

[owagner]

loop_starts() starts a sub thread which effectively calls loop_forever(), which in turn calls loop() in a, erm, tight loop. The subscribe events are fired and the messagehandler is called from that sub-thread, without delay, as soon as a message arrives. Isn't this actually your intention? The polling is handled from the main thread independently.

The explicit loop does something similar, but it also (I think unintentionally) may increase the polling frequency resolution to 1.25s (1s loop() timeout plus 0.25 explicit sleep)

I suggest to just revert the loop to the previous loop_start() approach

[nzfarmer1]

Feel free to revert but please test both options first. I'd be interested to know how you get on.

[nzfarmer1]

The thing to remember is that if you are running the code on a single threaded CPU you are relying on the OS to time splice efficiently between two tight loops. Personally I prefer a little more control.

[owagner]

Another question: You've added --clientid (good), but when setting it, you suffix it with the current time, presumbly to uniquify it. Doesn't this kind-of defeat the idea of specifiying an explicit clientid, or am I overlooking something?

[nzfarmer1]

Your are completely correct. The confusion is not lost on me. From experience I've learnt that non unique clientIds cause problems. We could call it clientidprefix, but that's a little long. Or remove it, but it can be handly to have something in the MQTT logs that's easily recognisable. Up to you.