[feature request] ModBus Connection status

[r-xyz]

Dear @tjhowse ,
Thanks for this nice piece of software. :)

I would like to improve the information regarding modbus connection status. This might help to use it as availability topics, as well as enabling clients to detect failures directly from MQTT data.
Before proceeding to a PR (if the repository is still active) I would like to check with you the best way to implement this.

Current status

1. On startup, the software attempts in sequence:
   1. Mobdus connection: if it fails, the system logs an error and exits. No failure message is published to MQTT.
   2. If the above succeeds, MQTT connection is attempted.
      1. If this fails due to auth problems, the system will keep retrying the connection.
      2. If an exception occurs (e.g. hostname do not resolve), the program exits with error.
      3. If all is ok, modbus4mqtt v connected is published under <prefix>/modbus4mqtt .
2. No message is published on disconnection
3. No message is published if -once initially connected- Modbus fail to poll registries or fails to reconnect.

Suggested solutions (feedback welcome)

1. Add last will message to notify MQTT disconnection (or program failing/being terminated and not running anymore) This should cover both n. 1.ii.a and 2. It could be done by adding the following before calling connect() on MQTT client. Content can actually be changed to just an empty string.

   self._mqtt_client.will_set(self.prefix+'modbus4mqtt', 'modbus4mqtt v{} disconnected.'.format(version.version))

2. Add additional topic to notify n. 3. It can be something like:

• At the end of connect_modbus (successful connection), adding a conditional statement to check that MQTT is connected.

  self._mqtt_client.will_set(self.prefix+'modbus', 'online')

• on poll() failure:

  self._mqtt_client.will_set(self.prefix+'modbus', 'offline')

3. Consider swapping connect_modbus() and connect_mqtt() at startup to avoid the extra conditional above. Not sure about implications.
4. Consider adding retain flag to both self.prefix+'modbus'and self.prefix+'modbus4mqtt'

Looking forward for any feedback to contribute.
Thanks in advance.

[pki791]

Hi, you can look into my fork, i added a timestamp to each message and i use it to che k if the data is fresh 😁

[tjhowse]

Hi @r-xyz ,

Those improvements all seem very reasonable! However I would suggest a few small changes:

Item 2: It is possible that an existing user has defined a register named "modbus" and that would conflict with the proposed online/offline status message. I would suggest publishing these messages to <prefix>/modbus4mqtt/modbus_status, or something similar. Also, the status message contents could provide a timestamp, as @pki791 has done, to ensure a stale online message can be detected by a consumer of the message? Perhaps a message like {"status": "online", "timestamp": "2024-09-24T01:23:45Z"}?

Item 3: Could you please explain the benefits of this change in more detail?

I'm assuming the will_set() calls in the online/offline code snippets are just a typo? Should they be publish()?

Please note that the best PRs tick the following boxes:

• Updated docs,
• Minimal code changes,
• Test coverage,
• Preserves backwards compatibility.

These help keep a FOSS project humming along smoothly.

Cheers,
tjhowse.

[r-xyz]

Hi @tjhowse and @pki791 ,
Thanks for the feedback.

Following up:
Item 1: OK
Item 2:

• I agree with the topic change proposed, did not though about the possible overlap.
• will_set() in this case was indeed a typo, apologies. I meant publish().
• I can add timestamp, though this will only show last time the program connected to/disconnected from the server (not last time any data was sent). I think this should be expected behavior. I would go for ISO 8601 format -as suggested.

Item 3:

• Currently, if modbus connection fails on startup, the program will be stuck in this loop before connecting to MQTT and no message will be sent to the server, even if the latter might have been available.
• Swapping connect_modbus() after connect_mqtt() in self.connect() would provide better feedback via MQTT.
• I agree this might be a substantial change, so we would need to check there will be no side effects.

Item 4:

• modbus4mqtt will become indeed a Last Will Topic, and modbus4mqtt/modbus_status will also have similar aim. In my understanding, they should be preferably retained by default. Otherwise I could add an lwt_retain parameter.

Will hopefully send a PR in the upcoming days.
Cheers,
r-xyz

[r-xyz]

If you prefer, I can also change modbus4mqtt message to JSON like that

{"status": "offline/online", "version":  "v0.7.0","timestamp": "2024-09-24T01:23:45Z"}

[tjhowse]

Thanks for your patience on this one – I've been busy with other work. I should have some time to do the review soon.

The format of the "modbus4mqtt v{} connected" message will have to remain unchanged until we do a major version number increase, as there's a good chance of breaking something. Adding the "modbus4mqtt v{} disconnected" message has some risk, but low enough, I feel.

Cheers,
tjhowse.

[r-xyz]

Thanks and no worries. :)
I could alternatively leave modbus4mqtt as it is, and add LWT in JSON format in modbus4mqtt/status.
It should not add much overhead to the MQTT server (modbus4mqtt is only published on connection), and this way you can decide if deprecating the old format/topic in the next major release.