shell: mqtt: use topic levels

[jeppenodgaard]

Change topic from <device_id>_rx (and tx) to <device_id>/sh/rx.

This allows use of wildcards. E.g. subscribe to all devices "+/sh/tx".

Level "sh" is added to the topic to make it less generic and prevent potential clashes with other topics.

[kartben]

I don't mind the change but this will be breaking users so needs to be mentioned in the migration guide.

Bonus points if:

• you consider adding some documentation for the MQTT backend in https://docs.zephyrproject.org/latest/services/shell/index.html#backends (of course this would be a different PR, not asking you to do it here)
• you consider making the topics being used something that can be configured via Kconfig, since things are pretty inflexible at the moment, as your PR basically highlights. I would argue this could/should be done in this very PR since it would effectively make this change non-breaking for folks if you keep the same default value as before.

Thanks!

[jeppenodgaard]

I don't mind the change but this will be breaking users so needs to be mentioned in the migration guide.

Bonus points if:

* you consider adding some documentation for the MQTT backend in https://docs.zephyrproject.org/latest/services/shell/index.html#backends (of course this would be a different PR, not asking you to do it here)

* you consider making the topics being used something that can be configured via Kconfig, since things are pretty inflexible at the moment, as your PR basically highlights. I would argue this could/should be done in this very PR since it would effectively make this change non-breaking for folks if you keep the same default value as before.

Thanks!

Since 4.2 hard freeze is tomorrow and there no 4.3 release files I created #92961

[jeppenodgaard]

Thank you for your review and insightful feedback @kartben .

you consider making the topics being used something that can be configured via Kconfig, since things are pretty inflexible at the moment, as your PR basically highlights. I would argue this could/should be done in this very PR since it would effectively make this change non-breaking for folks if you keep the same default value as before.

I think it is difficult to make something that is backward compatible and still follow MQTT best practice.

I would like to have SHELL_MQTT_TOPIC_IDENTIFIER which is default sh, SHELL_MQTT_TOPIC_TX_IDENTIFIER which is default tx and SHELL_MQTT_TOPIC_RX_IDENTIFIER which is default rx. This approach does not allow using _ between device identifier and the rest of the topic.

The backward compatible solution would be to make only two Kconfig which would have shared MQTT levels:

• SHELL_MQTT_TOPIC_RX_IDENTIFIER with default /sh/rx
• SHELL_MQTT_TOPIC_TX_IDENTIFIER with default /sh/tx

Which is a bit redundant.

Semi-related: I think there is a need for a common MQTT topic device identifier in Kconfig somewhere, so everything that uses MQTT has the same identifier.

[jeppenodgaard]

@kartben does my comment make any sense?

[jeppenodgaard]

The backward compatible solution would be to make only two Kconfig which would have shared MQTT levels:

• SHELL_MQTT_TOPIC_RX_IDENTIFIER with default /sh/rx
• SHELL_MQTT_TOPIC_TX_IDENTIFIER with default /sh/tx

I went with this.

It is a bit redundant but it allows backward compatibility and also allows adding another level e.g. /sh/1/rx, /sh/2/rx.

[jeppenodgaard]

@kartben can you please revisit?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kartben]

Nice! Sorry for the delay in coming back to this and thank you for your patience

[jeppenodgaard]

kartben's request has been resolved so this PR is ready to revisit @jakub-uC :)