diff --git a/src/paho/mqtt/client.py b/src/paho/mqtt/client.py index ed16a632..6143ef2a 100644 --- a/src/paho/mqtt/client.py +++ b/src/paho/mqtt/client.py @@ -751,9 +751,11 @@ def host(self) -> str: @host.setter def host(self, value: str) -> None: """ - Update host. This will only be used on future (re)connection. You should probably - use reconnect() to update the connection if established. + Update host. This may not be called if the connection is already open. """ + if not self._connection_closed(): + raise RuntimeError("updating host on established connection is not supported") + if not value: raise ValueError("Invalid host.") self._host = value @@ -766,9 +768,11 @@ def port(self) -> int: @port.setter def port(self, value: int) -> None: """ - Update port. This will only be used on future (re)connection. You should probably - use reconnect() to update the connection if established. + Update port. This may not be called if the connection is already open. """ + if not self._connection_closed(): + raise RuntimeError("updating port on established connection is not supported") + if value <= 0: raise ValueError("Invalid port number.") self._port = value @@ -781,7 +785,7 @@ def keepalive(self) -> int: @keepalive.setter def keepalive(self, value: int) -> None: """Update the client keepalive interval. This may not be called if the connection is already open.""" - if self._sock is not None: + if not self._connection_closed(): # The issue here is that the previous value of keepalive matter to possibly # sent ping packet. raise RuntimeError("updating keepalive on established connection is not supported") @@ -800,9 +804,11 @@ def transport(self) -> Literal["tcp", "websockets"]: def transport(self, value: Literal["tcp", "websockets"]) -> None: """ Update transport which should be "tcp" or "websockets". - This will only be used on future (re)connection. You should probably - use reconnect() to update the connection if established. + This may not be called if the connection is already open. """ + if not self._connection_closed(): + raise RuntimeError("updating transport on established connection is not supported") + self._transport = value @property @@ -817,7 +823,10 @@ def connect_timeout(self) -> float: @connect_timeout.setter def connect_timeout(self, value: float) -> None: - "Change connect_timeout for future (re)connection" + "Change connect_timeout. This may not be called if the connection is already open." + if not self._connection_closed(): + raise RuntimeError("updating connect_timeout on established connection is not supported") + if value <= 0.0: raise ValueError("timeout must be a positive number") @@ -833,9 +842,11 @@ def username(self) -> str | None: @username.setter def username(self, value: str | None) -> None: """ - Update username. This will only be used on future (re)connection. You should probably - use reconnect() to update the connection if established. + Update username. This may not be called if the connection is already open. """ + if not self._connection_closed(): + raise RuntimeError("updating username on established connection is not supported") + if value is None: self._username = None else: @@ -851,9 +862,11 @@ def password(self) -> str | None: @password.setter def password(self, value: str | None) -> None: """ - Update password. This will only be used on future (re)connection. You should probably - use reconnect() to update the connection if established. + Update password. This may not be called if the connection is already open. """ + if not self._connection_closed(): + raise RuntimeError("updating password on established connection is not supported") + if value is None: self._password = None else: @@ -866,8 +879,8 @@ def max_inflight_messages(self) -> int: @max_inflight_messages.setter def max_inflight_messages(self, value: int) -> None: - "Update max_inflight_messages. It's behavior is undefined if the connection is already open" - if self._sock is not None: + "Update max_inflight_messages. This may not be called if the connection is already open." + if not self._connection_closed(): # Not tested. Some doubt that everything is okay when max_inflight change between 0 # and > 0 value because _update_inflight is skipped when _max_inflight_messages == 0 raise RuntimeError("updating max_inflight_messages on established connection is not supported") @@ -884,8 +897,8 @@ def max_queued_messages(self) -> int: @max_queued_messages.setter def max_queued_messages(self, value: int) -> None: - "Update max_queued_messages. It's behavior is undefined if the connection is already open" - if self._sock is not None: + "Update max_queued_messages. This may not be called if the connection is already open." + if not self._connection_closed(): # Not tested. raise RuntimeError("updating max_queued_messages on established connection is not supported") @@ -1299,6 +1312,8 @@ def connect_async( connect call that can be used with loop_start() to provide very quick start. + Any already established connection will be terminated immediately. + host is the hostname or IP address of the remote broker. port is the network port of the server host to connect to. Defaults to 1883. Note that the default port for MQTT over SSL/TLS is 8883 so if you @@ -1316,6 +1331,10 @@ def connect_async( if bind_port < 0: raise ValueError('Invalid bind port number.') + # Switch to state NEW to allow update of host, port & co. + self._sock_close() + self._state = ConnectionState.MQTT_CS_NEW + self.host = host self.port = port self.keepalive = keepalive @@ -1651,6 +1670,14 @@ def enable_bridge_mode(self) -> None: """ self._client_mode = MQTT_BRIDGE + def _connection_closed(self) -> bool: + """ + Return true if the connection is closed (and not trying to be opened). + """ + return ( + self._state == ConnectionState.MQTT_CS_NEW + or (self._state in (ConnectionState.MQTT_CS_DISCONNECTING, ConnectionState.MQTT_CS_DISCONNECTED) and self._sock is None)) + def is_connected(self) -> bool: """Returns the current status of the connection diff --git a/tests/test_client.py b/tests/test_client.py index 361c5211..6c41fe93 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -94,6 +94,110 @@ def on_connect(mqttc, obj, flags, rc): finally: mqttc.loop_stop() + def test_connection_properties(self, proto_ver, fake_broker): + mqttc = client.Client("client-id", protocol=proto_ver) + mqttc.enable_logger() + + is_connected = threading.Event() + is_disconnected = threading.Event() + + def on_connect(mqttc, obj, flags, rc): + assert rc == 0 + is_connected.set() + + def on_disconnect(*args): + import logging + logging.info("disco") + is_disconnected.set() + + mqttc.on_connect = on_connect + mqttc.on_disconnect = on_disconnect + + mqttc.host = "localhost" + mqttc.connect_timeout = 7 + mqttc.port = fake_broker.port + mqttc.keepalive = 7 + mqttc.max_inflight_messages = 7 + mqttc.max_queued_messages = 7 + mqttc.transport = "tcp" + mqttc.username = "username" + mqttc.password = "password" + + mqttc.reconnect() + + # As soon as connection try to be established, no longer accept updates + with pytest.raises(RuntimeError): + mqttc.host = "localhost" + + mqttc.loop_start() + + try: + fake_broker.start() + + connect_packet = paho_test.gen_connect( + "client-id", + keepalive=7, + username="username", + password="password", + proto_ver=proto_ver, + ) + packet_in = fake_broker.receive_packet(1000) + assert packet_in # Check connection was not closed + assert packet_in == connect_packet + + connack_packet = paho_test.gen_connack(rc=0) + count = fake_broker.send_packet(connack_packet) + assert count # Check connection was not closed + assert count == len(connack_packet) + + is_connected.wait() + + # Check that all connections related properties can't be updated + with pytest.raises(RuntimeError): + mqttc.host = "localhost" + + with pytest.raises(RuntimeError): + mqttc.connect_timeout = 7 + + with pytest.raises(RuntimeError): + mqttc.port = fake_broker.port + + with pytest.raises(RuntimeError): + mqttc.keepalive = 7 + + with pytest.raises(RuntimeError): + mqttc.max_inflight_messages = 7 + + with pytest.raises(RuntimeError): + mqttc.max_queued_messages = 7 + + with pytest.raises(RuntimeError): + mqttc.transport = "tcp" + + with pytest.raises(RuntimeError): + mqttc.username = "username" + + with pytest.raises(RuntimeError): + mqttc.password = "password" + + # close the connection, but from broker + fake_broker.finish() + + is_disconnected.wait() + assert not mqttc.is_connected() + + # still not allowed to update, because client try to reconnect in background + with pytest.raises(RuntimeError): + mqttc.host = "localhost" + + mqttc.disconnect() + + # Now it's allowed, connection is closing AND not trying to reconnect + mqttc.host = "localhost" + + finally: + mqttc.loop_stop() + class Test_connect_v5: """