Address PR review comments

- client.py on_feature: move future.done() check inside call_soon_threadsafe
  callback to eliminate race between done-check and set_result across
  the SDK thread / event loop thread boundary
- tests/test_mqtt_reconnection.py: replace asyncio.Future with
  concurrent.futures.Future to match what the AWS IoT SDK actually
  returns, exercising the wrap_future() conversion path in close()
- encoding.py build_reservation_entry: read get_unit_system() once into
  a local variable instead of calling it twice

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: eman

src/nwp500/encoding.py

@@ -434,18 +434,21 @@ def build_reservation_entry(
     from .models import preferred_to_half_celsius
     from .unit_system import get_unit_system
 
+    # Read unit system once to keep min/max bounds consistent
+    unit_system = get_unit_system()
+
     # Use device-provided limits if available, otherwise use defaults
     # in the user's preferred unit system.
     if temperature_min is not None:
         min_temp = temperature_min
-    elif get_unit_system() == "metric":
+    elif unit_system == "metric":
         min_temp = 35.0  # ~35°C
     else:
         min_temp = 95.0  # 95°F
 
     if temperature_max is not None:
         max_temp = temperature_max
-    elif get_unit_system() == "metric":
+    elif unit_system == "metric":
         max_temp = 65.0  # ~65°C
     else:
         max_temp = 150.0  # 150°F

src/nwp500/mqtt/client.py

@@ -1300,10 +1300,16 @@ async def ensure_device_info_cached(
         future: asyncio.Future[DeviceFeature] = loop.create_future()
 
         def on_feature(feature: DeviceFeature) -> None:
-            # Called from AWS SDK thread — must use thread-safe method
-            if not future.done():
-                _logger.info(f"Device feature received for {redacted_mac}")
-                loop.call_soon_threadsafe(future.set_result, feature)
+            # Called from AWS SDK thread — schedule onto the event loop
+            # thread-safely. The done() check is inside the scheduled
+            # callback so it runs on the event loop thread, eliminating
+            # the race between the check and set_result.
+            def _set_result() -> None:
+                if not future.done():
+                    _logger.info(f"Device feature received for {redacted_mac}")
+                    future.set_result(feature)
+
+            loop.call_soon_threadsafe(_set_result)
 
         _logger.info(f"Ensuring device info cached for {redacted_mac}")
         await self.subscribe_device_feature(device, on_feature)

tests/test_mqtt_reconnection.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-import asyncio
+import concurrent.futures
 from unittest.mock import MagicMock
 
 import pytest
@@ -40,6 +40,16 @@ def config():
     return MqttConnectionConfig(client_id="test-client")
 
 
+def _make_cf_future(result=None, exception=None):
+    """Return a resolved concurrent.futures.Future matching SDK behaviour."""
+    f = concurrent.futures.Future()
+    if exception is not None:
+        f.set_exception(exception)
+    else:
+        f.set_result(result)
+    return f
+
+
 class TestMqttConnectionClose:
     """Tests for MqttConnection.close() method."""
 
@@ -55,15 +65,14 @@ async def test_close_on_none_connection(self, config, mock_auth_client):
 
     @pytest.mark.asyncio(loop_scope="function")
     async def test_close_clears_state(self, config, mock_auth_client):
-        """close() should clear _connection and _connected regardless."""
+        """close() should clear _connection and _connected regardless.
+
+        Uses concurrent.futures.Future to match what the AWS IoT SDK
+        returns from disconnect(), exercising the wrap_future() path.
+        """
         conn = MqttConnection(config, mock_auth_client)
-        # Simulate a connection that was interrupted (_connected=False
-        # but _connection still exists)
         mock_sdk_conn = MagicMock()
-        loop = asyncio.get_running_loop()
-        future = loop.create_future()
-        future.set_result(None)
-        mock_sdk_conn.disconnect.return_value = future
+        mock_sdk_conn.disconnect.return_value = _make_cf_future()
         conn._connection = mock_sdk_conn
         conn._connected = False  # Interrupted state
 
@@ -97,16 +106,13 @@ async def test_close_handles_already_dead_connection(
 
         conn = MqttConnection(config, mock_auth_client)
         mock_sdk_conn = MagicMock()
-        loop = asyncio.get_running_loop()
-        future = loop.create_future()
-        future.set_exception(
-            AwsCrtError(
+        mock_sdk_conn.disconnect.return_value = _make_cf_future(
+            exception=AwsCrtError(
                 code=0,
                 name="AWS_ERROR_MQTT_CONNECTION_DESTROYED",
                 message="Connection destroyed",
             )
         )
-        mock_sdk_conn.disconnect.return_value = future
         conn._connection = mock_sdk_conn
         conn._connected = False