modbus: serial: Allow custom stop-bit settings in server mode

[msalau]

A Modbus server should be able to operate with the same settings as a client. Stop-bits -is the only setting not available in server mode.

The change makes it possible to override default stop-bits settings in backwards compatible way. The feature is disabled by default.

[jfischer-no]

A Modbus server should be able to operate with the same settings as a
client.

Why should it?

Stop-bits -is the only setting not available in server mode.

Because the RTU format is always 11 bits. There is either parity + stop or stop + stop. The client only supports this to communicate with non-compliant server devices, introduced in #40558.

[msalau]

Hello @jfischer-no
Thank you for feedback!
Yes, I'm fully aware that 8N1 is not compliant with the specification but there are devices on the market that use exactly this format.
Right now the Modbus subsystem can talk to a non-compliant server device as a client but can't act as a non-compliant server. So we can't create a server device that may be added to an existing non-compliant bus.
This is exactly my case: an existing bus that works at 9600 8N1 and I need to add another server to it without changing the communication format.

May be a better description of the config option is needed to highlight that this options is not compliant with the specification and should be used with caution.

[jfischer-no]

understood, I am fine with

config MODBUS_NONCOMPLIANT_RTU_MODE
	bool "Non-compliant RTU server mode"
	depends on MODBUS_SERVER && MODBUS_SERIAL
	help
	  Allow non-compliant stop and parity bit settings.

[jfischer-no]

depends on MODBUS_SERVER && MODBUS_SERIAL

[jfischer-no]

understood, I am fine with

config MODBUS_NONCOMPLIANT_RTU_MODE
	bool "Non-compliant RTU server mode"
	depends on MODBUS_SERVER && MODBUS_SERIAL
	help
	  Allow non-compliant stop and parity bit settings.

[msalau]

Hello @jfischer-no
Thank you for feedback. I've updated the change according to your comment.
I've renamed the option from MODBUS_NONCOMPLIANT_RTU_MODE to MODBUS_NONCOMPLIANT_SERIAL_SERVER_MODE since it affects not just RTU but ASCII also.

What about renaming struct modbus_serial_param::stop_bits_client into stop_bits and making non-compliant stop-bit configuration disabled by default for both clients and servers?
The current implementation allows non-compliant stop-bits configuration at all times for clients.

I.e.:
modbus/Kconfig:

config MODBUS_NONCOMPLIANT_SERIAL_MODE
	bool "Non-compliant serial mode"
	depends on MODBUS_SERIAL
	help
	  Allow non-compliant stop and parity bit settings.

modbus/modbus.h:

/**
 * @brief Modbus serial line parameter
 */
struct modbus_serial_param {
	/** Baudrate of the serial line */
	uint32_t baud;
	/** parity UART's parity setting:
	 *    UART_CFG_PARITY_NONE,
	 *    UART_CFG_PARITY_EVEN,
	 *    UART_CFG_PARITY_ODD
	 */
	enum uart_config_parity parity;
-	/** stop_bits_client UART's stop bits setting if in client mode:
+	/** stop_bits UART's stop bits setting:
	 *    UART_CFG_STOP_BITS_0_5,
	 *    UART_CFG_STOP_BITS_1,
	 *    UART_CFG_STOP_BITS_1_5,
	 *    UART_CFG_STOP_BITS_2,
	 */
-	enum uart_config_stop_bits stop_bits_client;
+	enum uart_config_stop_bits stop_bits;
};

modbus/modbus_serial.c:

-	if (ctx->client || IS_ENABLED(MODBUS_NONCOMPLIANT_SERIAL_SERVER_MODE)) {
+ 	if (IS_ENABLED(CONFIG_MODBUS_NONCOMPLIANT_SERIAL_MODE)) {
		/* Allow custom stop bit settings */
-		switch (param->serial.stop_bits_client) {
+		switch (param->serial.stop_bits) {
		case UART_CFG_STOP_BITS_0_5:
		case UART_CFG_STOP_BITS_1:
		case UART_CFG_STOP_BITS_1_5:
		case UART_CFG_STOP_BITS_2:
			uart_cfg.stop_bits = param->serial.stop_bits;
			break;
		default:
			return -EINVAL;
		}
	}

This will require some migration effort for existing applications but will explicitly enable non-compliant mode for both clients and servers.
For compile-time validation of applications the stop_bits field may be disabled using #ifdef..#endif

[jfischer-no]

Hello @jfischer-no Thank you for feedback. I've updated the change according to your comment. I've renamed the option from MODBUS_NONCOMPLIANT_RTU_MODE to MODBUS_NONCOMPLIANT_SERIAL_SERVER_MODE since it affects not just RTU but ASCII also.

What about renaming struct modbus_serial_param::stop_bits_client into stop_bits and making non-compliant stop-bit configuration disabled by default for both clients and servers? The current implementation allows non-compliant stop-bits configuration at all times for clients.

I.e.: modbus/Kconfig:

config MODBUS_NONCOMPLIANT_SERIAL_MODE
	bool "Non-compliant serial mode"
	depends on MODBUS_SERIAL
	help
	  Allow non-compliant stop and parity bit settings.

Yes, that is event better.

[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

[jfischer-no]

LGTM, thanks.