From 3ce3a5eb1a0ba65bc6345ac696ccd5c0e508cdcc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Prus-Zaj=C4=85czkowski?=
 <bartlomiej.prus@brainly.com>
Date: Thu, 13 Aug 2026 17:12:26 +0200
Subject: [PATCH 1/4] Support writing bit fields via read-modify-write

Modbus has no bit write for holding registers, so a register that packs
several independent controls could be read (`bits` / `shift_bits`) but
never written. Writing one meant a read-modify-write loop outside the
integration, which is not atomic against the coordinator's own polling.

Writing a field declared with `bits` / `shift_bits` now reads the
register, replaces just that field and writes it back, all inside the
existing client lock - the same lock `update_device()` takes - so no
poll or other write on that gateway can land in between. This does not
help against a second Modbus master, but it closes the window this
integration was creating itself.

No new YAML syntax: `bits` / `shift_bits` on a writable control already
describe exactly the field geometry the merge needs.

- conversion: `merge_into_registers()` merges a descaled field into the
  registers read from the device, and `field_geometry()` resolves
  shift/mask. `_descale()` is split out of `_convert_from_decimal()` so
  scaled fields (`multiplier`) work. `_swap_registers` is its own
  inverse, so the same call un-swaps and re-swaps.
- tcp_client: `_read_current_registers()` reads the whole field in one
  transaction rather than honouring `max_register_read` chunking - a
  field split across two reads could tear. A failed read raises and
  aborts the write; merging onto a guess would clear the field's
  neighbours, which is worse than not writing.
- A value that does not fit its mask raises rather than truncating, for
  the same reason.
- modbus_device_info: switches on a bit field are no longer rejected.
  `number` and `select` already built and only failed at write time.
  Coils still reject bits, where they are meaningless.
- base: writable bit fields reject `signed` and `sum_scale`, whose
  arithmetic the merge cannot express. Deliberately not applied to
  sensors - tightening those would stop entities that work today from
  being created at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../modbus_local_gateway/conversion.py        |  86 ++++++++-
 .../entity_management/base.py                 |  43 +++++
 .../entity_management/modbus_device_info.py   |   4 -
 .../modbus_local_gateway/tcp_client.py        |  45 ++++-
 tests/test_client.py                          | 128 ++++++++++++++
 tests/test_conversion.py                      | 164 +++++++++++++++++-
 tests/test_entity_management_base.py          |  77 ++++++++
 tests/test_modbus_device_info.py              |  19 +-
 8 files changed, 544 insertions(+), 22 deletions(-)

diff --git a/custom_components/modbus_local_gateway/conversion.py b/custom_components/modbus_local_gateway/conversion.py
index 346efc0..7446f08 100644
--- a/custom_components/modbus_local_gateway/conversion.py
+++ b/custom_components/modbus_local_gateway/conversion.py
@@ -200,27 +200,95 @@ def _apply_conversion_operations(
 
         return num
 
-    def _convert_from_decimal(
-        self, num: float, desc: ModbusEntityDescription
-    ) -> list[int]:
-        """Convert from a decimal to registers"""
+    def _descale(self, num: float, desc: ModbusEntityDescription) -> int:
+        """Reverse multiplier/offset and round to the raw register value.
+
+        The inverse of `_apply_conversion_operations`.
+        """
         if desc.conv_offset:
             num -= desc.conv_offset
         if desc.conv_multiplier is not None:
             num = num / desc.conv_multiplier
-        if desc.conv_bits:
-            raise NotSupportedError("Setting of bit fields is not supported")
-        if desc.conv_shift_bits:
-            raise NotSupportedError("Setting of bit fields is not supported")
+        return int(round(num))
+
+    def field_geometry(self, desc: ModbusEntityDescription) -> tuple[int, int]:
+        """Return (shift, mask) for a bit field.
+
+        When `bits` is omitted the field is taken to run from `shift_bits` up to
+        the top of the register span, which matches how the read path behaves.
+        """
+        shift: int = desc.conv_shift_bits or 0
+        width: int = desc.conv_bits or (16 * (desc.register_count or 1) - shift)
+        return shift, (1 << width) - 1
+
+    def _convert_from_decimal(
+        self, num: float, desc: ModbusEntityDescription
+    ) -> list[int]:
+        """Convert from a decimal to registers"""
+        if desc.conv_bits or desc.conv_shift_bits:
+            raise NotSupportedError(
+                "Bit fields must be written with merge_into_registers"
+            )
         if desc.conv_sum_scale:
             raise NotSupportedError("Setting of scaled sums is not supported")
 
         registers: list[int] = self.client.convert_to_registers(
-            int(round(num)),
+            self._descale(num, desc),
             data_type=self._get_number_data_type(desc),
         )
         return registers
 
+    def merge_into_registers(
+        self,
+        desc: ModbusEntityDescription,
+        value: float,
+        current_registers: list[int],
+    ) -> list[int]:
+        """Merge a bit-field value into the register(s) currently on the device.
+
+        Modbus cannot write individual bits of a holding register, so writing a
+        field declared with `bits` / `shift_bits` means reading the whole
+        register, replacing just that field, and writing it back. The caller is
+        responsible for doing the read and the write under the client lock so
+        the pair is atomic.
+
+        `_swap_registers` is its own inverse for every supported swap type, so
+        the same call un-swaps on the way in and re-swaps on the way out.
+        """
+        if desc.conv_sum_scale:
+            raise NotSupportedError("Setting of scaled sums is not supported")
+
+        data_type = self._get_number_data_type(desc)
+        raw = self.client.convert_from_registers(
+            self._swap_registers(current_registers, desc), data_type=data_type
+        )
+        if not isinstance(raw, int):
+            raise InvalidDataTypeError(
+                f"Invalid data type for bit field merge: {type(raw).__name__}"
+            )
+
+        shift, mask = self.field_geometry(desc)
+        field: int = self._descale(value, desc)
+        if not 0 <= field <= mask:
+            raise ValueError(
+                f"Value {value} maps to {field}, which does not fit the "
+                f"{mask.bit_length()}-bit field at shift {shift} of {desc.key}"
+            )
+
+        merged: int = (raw & ~(mask << shift)) | (field << shift)
+        _LOGGER.debug(
+            "Merging %s into %s: 0x%04X -> 0x%04X (mask 0x%X << %d)",
+            field,
+            desc.key,
+            raw,
+            merged,
+            mask,
+            shift,
+        )
+        return self._swap_registers(
+            self.client.convert_to_registers(merged, data_type=data_type), desc
+        )
+
     def convert_from_response(
         self, desc: ModbusEntityDescription, response: ModbusPDU
     ) -> str | float | int | bool | None:
diff --git a/custom_components/modbus_local_gateway/entity_management/base.py b/custom_components/modbus_local_gateway/entity_management/base.py
index 6c4b7bb..7041e65 100644
--- a/custom_components/modbus_local_gateway/entity_management/base.py
+++ b/custom_components/modbus_local_gateway/entity_management/base.py
@@ -101,6 +101,49 @@ def validate(self) -> bool:
             return False
         if not self._validate_scan_interval():
             return False
+        if not self._validate_bitfield():
+            return False
+        return True
+
+    def _validate_bitfield(self) -> bool:
+        """Check constraints for writable bit fields.
+
+        Writing a `bits` / `shift_bits` field is a read-modify-write, which
+        merges the field into the register currently on the device. That
+        arithmetic assumes an unsigned value and a single register span.
+
+        Deliberately limited to the writable control types: applying it to
+        sensors would stop already-working entities from being created.
+        """
+        if not (self.conv_bits or self.conv_shift_bits):
+            return True
+        if self.control_type not in (
+            ControlType.NUMBER,
+            ControlType.SWITCH,
+            ControlType.SELECT,
+        ):
+            return True
+
+        if self.is_signed:
+            _LOGGER.warning(
+                "Unable to create entity for %s: %s cannot be combined with "
+                "%s or %s on a writable entity",
+                self.key,
+                IS_SIGNED,
+                CONV_BITS,
+                CONV_SHIFT_BITS,
+            )
+            return False
+        if self.conv_sum_scale:
+            _LOGGER.warning(
+                "Unable to create entity for %s: %s cannot be combined with "
+                "%s or %s on a writable entity",
+                self.key,
+                CONV_SUM_SCALE,
+                CONV_BITS,
+                CONV_SHIFT_BITS,
+            )
+            return False
         return True
 
     def _validate_scan_interval(self) -> bool:
diff --git a/custom_components/modbus_local_gateway/entity_management/modbus_device_info.py b/custom_components/modbus_local_gateway/entity_management/modbus_device_info.py
index d043aec..aba4938 100644
--- a/custom_components/modbus_local_gateway/entity_management/modbus_device_info.py
+++ b/custom_components/modbus_local_gateway/entity_management/modbus_device_info.py
@@ -345,10 +345,6 @@ def _handle_switch_description(
     ) -> None | type[ModbusSwitchEntityDescription]:
         """Handle switch description specific logic"""
         switch_data = _data.get("switch", {})
-        if _data.get(CONV_BITS) or _data.get(CONV_SHIFT_BITS):
-            _LOGGER.warning("bits / shift bits cannot be set for Switches")
-            return None
-
         if not isinstance(switch_data, dict):
             _LOGGER.warning(
                 "Switch configuration for %s should be a dictionary", entity
diff --git a/custom_components/modbus_local_gateway/tcp_client.py b/custom_components/modbus_local_gateway/tcp_client.py
index 83542db..5f709d4 100644
--- a/custom_components/modbus_local_gateway/tcp_client.py
+++ b/custom_components/modbus_local_gateway/tcp_client.py
@@ -213,6 +213,35 @@ async def _write_registers_individually(
                 return
         _LOGGER.debug("All individual writes successful using fallback")
 
+    async def _read_current_registers(self, entity: ModbusContext) -> list[int]:
+        """Read the register(s) backing a bit field, for a read-modify-write.
+
+        Must be called with the client lock held so the read and the following
+        write cannot be interleaved with a poll.
+
+        The whole span is read in a single transaction (`max_read_size` is the
+        field's own size) rather than honouring the device's `max_register_read`
+        chunking: a field split across two reads could tear if the device
+        changed in between. `register_count` is at most 4 and the default chunk
+        is 8, so this only differs for a gateway that cannot read the field in
+        one go - and there it fails loudly and aborts the write, which is the
+        safe outcome.
+        """
+        response: ModbusPDU | None = await self.read_data(
+            func=self.read_holding_registers,
+            address=entity.desc.register_address,
+            count=entity.desc.register_count,
+            device_id=entity.device_id,
+            max_read_size=entity.desc.register_count,
+        )
+        if response is None or response.isError():
+            raise ModbusException(
+                "Unable to read current value of "
+                f"{entity.desc.key} at {entity.desc.register_address} - "
+                "aborting bit field write"
+            )
+        return response.registers
+
     async def write_data(self, entity: ModbusContext, value: Any) -> ModbusPDU | None:
         """Writes data to Holding Registers or Coils"""
         pdu: ModbusPDU | None = None
@@ -236,9 +265,19 @@ async def write_data(self, entity: ModbusContext, value: Any) -> ModbusPDU | Non
             )
 
             if entity.desc.data_type == ModbusDataType.HOLDING_REGISTER:
-                registers = Conversion(type(self)).convert_to_registers(
-                    entity.desc, value
-                )
+                conversion = Conversion(type(self))
+                if entity.desc.conv_bits or entity.desc.conv_shift_bits:
+                    # Modbus has no bit write for holding registers: read the
+                    # whole register, replace just this field, write it back.
+                    # Still inside `self.lock`, so no poll or other write can
+                    # land between the read and the write.
+                    registers = conversion.merge_into_registers(
+                        entity.desc,
+                        value,
+                        await self._read_current_registers(entity),
+                    )
+                else:
+                    registers = conversion.convert_to_registers(entity.desc, value)
                 _LOGGER.debug(
                     "Raw value after conversion to registers: %s (type: %s)",
                     registers,
diff --git a/tests/test_client.py b/tests/test_client.py
index 7ae49fa..83ddf96 100644
--- a/tests/test_client.py
+++ b/tests/test_client.py
@@ -934,3 +934,131 @@ async def test_write_data_invalid_coil_value_type() -> None:
         pytest.raises(TypeError, match="Value for COIL must be boolean, got int"),
     ):
         await client.write_data(entity, value=123)
+
+
+def _bitfield_entity() -> ModbusContext:
+    """A switch on bit 4 of a holding register."""
+    return ModbusContext(
+        device_id=1,
+        desc=ModbusSwitchEntityDescription(
+            key="bitfield",
+            register_address=1,
+            register_count=1,
+            control_type="switch",
+            data_type=ModbusDataType.HOLDING_REGISTER,
+            conv_bits=1,
+            conv_shift_bits=4,
+            on=1,
+            off=0,
+        ),
+    )
+
+
+@pytest.mark.asyncio
+async def test_write_data_bitfield_read_modify_write() -> None:
+    """Writing a bit field reads the register and merges into it."""
+    client = AsyncModbusTcpClientGateway(host="localhost")
+    client.connect = AsyncMock()
+    client.write_register = AsyncMock(return_value=ModbusPDU())
+    client.write_register.return_value.isError = lambda: False
+
+    with (
+        patch.object(
+            AsyncModbusTcpClientGateway, "connected", PropertyMock(return_value=True)
+        ),
+        patch.object(
+            AsyncModbusTcpClientGateway,
+            "read_data",
+            AsyncMock(
+                return_value=ReadHoldingRegistersResponse(registers=[0b0000_0011])
+            ),
+        ),
+    ):
+        await client.write_data(_bitfield_entity(), value=1)
+
+        # bit 4 set, the two bits already on are untouched
+        client.write_register.assert_called_once_with(
+            address=1,
+            value=0b0001_0011,
+            device_id=1,
+        )
+
+
+@pytest.mark.asyncio
+async def test_write_data_bitfield_read_failure_aborts_write() -> None:
+    """A failed read must abort - merging onto a guess would clear the field's
+    neighbours, which is worse than not writing at all."""
+    client = AsyncModbusTcpClientGateway(host="localhost")
+    client.connect = AsyncMock()
+    client.write_register = AsyncMock(return_value=ModbusPDU())
+
+    with (
+        patch.object(
+            AsyncModbusTcpClientGateway, "connected", PropertyMock(return_value=True)
+        ),
+        patch.object(
+            AsyncModbusTcpClientGateway, "read_data", AsyncMock(return_value=None)
+        ),
+        pytest.raises(ModbusException, match="aborting bit field write"),
+    ):
+        await client.write_data(_bitfield_entity(), value=1)
+
+    client.write_register.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_write_data_bitfield_error_response_aborts_write() -> None:
+    """An error PDU from the read is a failed read, not a value of zero."""
+    client = AsyncModbusTcpClientGateway(host="localhost")
+    client.connect = AsyncMock()
+    client.write_register = AsyncMock(return_value=ModbusPDU())
+
+    error_response = ReadHoldingRegistersResponse(registers=[0])
+    error_response.isError = lambda: True  # type: ignore[method-assign]
+
+    with (
+        patch.object(
+            AsyncModbusTcpClientGateway, "connected", PropertyMock(return_value=True)
+        ),
+        patch.object(
+            AsyncModbusTcpClientGateway,
+            "read_data",
+            AsyncMock(return_value=error_response),
+        ),
+        pytest.raises(ModbusException, match="aborting bit field write"),
+    ):
+        await client.write_data(_bitfield_entity(), value=1)
+
+    client.write_register.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_write_data_non_bitfield_does_not_read_first() -> None:
+    """Plain registers keep the single-transaction write they always had."""
+    client = AsyncModbusTcpClientGateway(host="localhost")
+    client.connect = AsyncMock()
+    client.write_register = AsyncMock(return_value=ModbusPDU())
+    client.write_register.return_value.isError = lambda: False
+
+    entity = ModbusContext(
+        device_id=1,
+        desc=ModbusEntityDescription(
+            key="plain",
+            register_address=1,
+            register_count=1,
+            data_type=ModbusDataType.HOLDING_REGISTER,
+        ),
+    )
+
+    with (
+        patch.object(
+            AsyncModbusTcpClientGateway, "connected", PropertyMock(return_value=True)
+        ),
+        patch.object(
+            AsyncModbusTcpClientGateway, "read_data", AsyncMock()
+        ) as read_data,
+    ):
+        await client.write_data(entity, value=123)
+
+        read_data.assert_not_called()
+        client.write_register.assert_called_once()
diff --git a/tests/test_conversion.py b/tests/test_conversion.py
index 6d33b8a..fccd4c4 100644
--- a/tests/test_conversion.py
+++ b/tests/test_conversion.py
@@ -4,15 +4,21 @@
 import pytest
 from pymodbus.client.mixin import ModbusClientMixin
 from pymodbus.pdu.bit_message import ReadCoilsResponse, ReadDiscreteInputsResponse
-from pymodbus.pdu.register_message import ReadInputRegistersResponse
+from pymodbus.pdu.register_message import (
+    ReadHoldingRegistersResponse,
+    ReadInputRegistersResponse,
+)
 
 from custom_components.modbus_local_gateway.conversion import (
     Conversion,
     InvalidDataTypeError,
+    NotSupportedError,
 )
 from custom_components.modbus_local_gateway.entity_management.base import (
     ModbusDataType,
+    ModbusNumberEntityDescription,
     ModbusSensorEntityDescription,
+    ModbusSwitchEntityDescription,
 )
 from custom_components.modbus_local_gateway.entity_management.const import SwapType
 from custom_components.modbus_local_gateway.tcp_client import AsyncModbusTcpClient
@@ -667,3 +673,159 @@ def test_get_float_type(size: int, expected: ModbusClientMixin.DATATYPE) -> None
     else:
         result: ModbusClientMixin.DATATYPE = conversion._get_float_data_type(desc)
         assert result == expected
+
+
+def _switch_desc(**kwargs) -> ModbusSwitchEntityDescription:
+    """A writable bit-field switch description."""
+    return ModbusSwitchEntityDescription(
+        register_address=1,
+        key="test",
+        control_type="switch",
+        data_type=ModbusDataType.HOLDING_REGISTER,
+        **kwargs,
+    )
+
+
+def _number_desc(**kwargs) -> ModbusNumberEntityDescription:
+    """A writable bit-field number description."""
+    return ModbusNumberEntityDescription(
+        register_address=1,
+        key="test",
+        control_type="number",
+        data_type=ModbusDataType.HOLDING_REGISTER,
+        min=0,
+        max=255,
+        **kwargs,
+    )
+
+
+@pytest.mark.parametrize(
+    ("current", "value", "expected"),
+    [
+        (0b0000_0000_0001_0011, 1, 0b0000_0000_0001_0011),  # already set, no change
+        (0b0000_0000_0000_0011, 1, 0b0000_0000_0001_0011),  # set, neighbours kept
+        (0b1111_1111_1111_1111, 0, 0b1111_1111_1110_1111),  # clear, neighbours kept
+        (0b0000_0000_0001_0000, 0, 0b0000_0000_0000_0000),  # clear the only bit
+    ],
+)
+def test_merge_single_bit(current: int, value: int, expected: int) -> None:
+    """Setting or clearing one bit must leave every other bit untouched."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _switch_desc(conv_bits=1, conv_shift_bits=4, on=1, off=0)
+
+    assert conversion.merge_into_registers(desc, value, [current]) == [expected]
+
+
+def test_merge_low_byte_preserves_high_byte() -> None:
+    """The packed-zone case: writing zone1 must not zero zone2.
+
+    A register holding 30 in the high byte and 6 in the low byte; writing 45
+    to the low byte must leave the high byte at 30.
+    """
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_bits=8, conv_shift_bits=0)
+
+    result = conversion.merge_into_registers(desc, 45, [(30 << 8) | 6])
+
+    assert result == [(30 << 8) | 45]
+    assert result[0] >> 8 == 30
+
+
+def test_merge_high_byte_preserves_low_byte() -> None:
+    """The mirror case: writing the high byte must not disturb the low byte."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_bits=8, conv_shift_bits=8)
+
+    result = conversion.merge_into_registers(desc, 30, [(12 << 8) | 45])
+
+    assert result == [(30 << 8) | 45]
+    assert result[0] & 0xFF == 45
+
+
+def test_merge_across_two_registers() -> None:
+    """A field spanning the 32-bit boundary of a two-register entity."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(register_count=2, conv_bits=8, conv_shift_bits=12)
+
+    current = AsyncModbusTcpClient.convert_to_registers(
+        0xABCD_1234, data_type=AsyncModbusTcpClient.DATATYPE.UINT32
+    )
+    result = conversion.merge_into_registers(desc, 0xFF, current)
+
+    merged = AsyncModbusTcpClient.convert_from_registers(
+        result, data_type=AsyncModbusTcpClient.DATATYPE.UINT32
+    )
+    assert merged == 0xABCF_F234
+
+
+@pytest.mark.parametrize(
+    "swap", [None, SwapType.BYTE, SwapType.WORD, SwapType.WORD_BYTE]
+)
+def test_merge_round_trips_through_swap(swap) -> None:
+    """Merging then reading back must return the value that was written.
+
+    `_swap_registers` is its own inverse, which is what lets the merge use the
+    same call to un-swap and re-swap.
+    """
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(
+        register_count=2, conv_bits=8, conv_shift_bits=8, conv_swap=swap
+    )
+
+    current = AsyncModbusTcpClient.convert_to_registers(
+        0x0000_0000, data_type=AsyncModbusTcpClient.DATATYPE.UINT32
+    )
+    merged = conversion.merge_into_registers(desc, 0x5A, current)
+
+    read_back = conversion.convert_from_response(
+        desc=desc,
+        response=ReadHoldingRegistersResponse(registers=merged),
+    )
+    assert read_back == 0x5A
+
+
+def test_merge_applies_multiplier_and_offset() -> None:
+    """A scaled bit field descales before it is packed."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_bits=8, conv_shift_bits=0, conv_multiplier=0.5)
+
+    # 21.5 degrees / 0.5 == 43 raw, merged into the low byte
+    assert conversion.merge_into_registers(desc, 21.5, [0xFF00]) == [0xFF00 | 43]
+
+
+@pytest.mark.parametrize("value", [256, -1])
+def test_merge_rejects_value_that_does_not_fit(value: int) -> None:
+    """Overflowing the field would corrupt the neighbouring controls."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_bits=8, conv_shift_bits=0)
+
+    with pytest.raises(ValueError, match="does not fit"):
+        conversion.merge_into_registers(desc, value, [0x1234])
+
+
+def test_merge_width_defaults_to_top_of_register() -> None:
+    """With `shift_bits` but no `bits`, the field runs to the top of the span."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_shift_bits=12)
+
+    assert conversion.field_geometry(desc) == (12, 0xF)
+    assert conversion.merge_into_registers(desc, 0xA, [0x5678]) == [0xA678]
+
+
+def test_convert_to_registers_still_refuses_bit_fields() -> None:
+    """The plain (non read-modify-write) path must not silently zero a field."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_bits=8, conv_shift_bits=0)
+
+    with pytest.raises(NotSupportedError, match="merge_into_registers"):
+        conversion.convert_to_registers(desc, 5)
+
+
+def test_merge_refuses_sum_scale() -> None:
+    """`sum_scale` has no inverse. Validation rejects it at load, but
+    merge_into_registers is public, so it guards too."""
+    conversion = Conversion(client=AsyncModbusTcpClient)
+    desc = _number_desc(conv_bits=8, conv_sum_scale=[1.0, 0.1])
+
+    with pytest.raises(NotSupportedError, match="scaled sums"):
+        conversion.merge_into_registers(desc, 5, [0x1234])
diff --git a/tests/test_entity_management_base.py b/tests/test_entity_management_base.py
index c3c2922..ebe2878 100644
--- a/tests/test_entity_management_base.py
+++ b/tests/test_entity_management_base.py
@@ -4,9 +4,12 @@
 # pylint: disable=unexpected-keyword-arg, protected-access
 from unittest.mock import patch
 
+import pytest
+
 from custom_components.modbus_local_gateway.entity_management.base import (
     ModbusEntityDescription,
 )
+from custom_components.modbus_local_gateway.entity_management.const import ControlType
 
 
 def test_validate_both_float_and_string(
@@ -106,3 +109,77 @@ def test_validate_valid_entity(
     ) as mock_warning:
         assert entity.validate()
         mock_warning.assert_not_called()
+
+
+@pytest.mark.parametrize(
+    "control_type", [ControlType.NUMBER, ControlType.SWITCH, ControlType.SELECT]
+)
+def test_validate_signed_bitfield_rejected_when_writable(
+    valid_entity_description: ModbusEntityDescription, control_type: str
+) -> None:
+    """A writable bit field cannot be signed - the merge assumes unsigned."""
+    entity: ModbusEntityDescription = valid_entity_description
+    entity = entity.__class__(
+        **{
+            **entity.__dict__,
+            "conv_bits": 8,
+            "is_signed": True,
+            "control_type": control_type,
+        }
+    )
+    with patch(
+        "custom_components.modbus_local_gateway.entity_management.base._LOGGER.warning"
+    ):
+        assert not entity.validate()
+
+
+def test_validate_signed_bitfield_allowed_on_sensor(
+    valid_entity_description: ModbusEntityDescription,
+) -> None:
+    """Read-only entities keep working: tightening them would delete entities
+    from configs that are valid today."""
+    entity: ModbusEntityDescription = valid_entity_description
+    entity = entity.__class__(
+        **{
+            **entity.__dict__,
+            "conv_bits": 8,
+            "is_signed": True,
+            "control_type": ControlType.SENSOR,
+        }
+    )
+    assert entity.validate()
+
+
+def test_validate_sum_scale_bitfield_rejected_when_writable(
+    valid_entity_description: ModbusEntityDescription,
+) -> None:
+    """`sum_scale` has no meaningful inverse, so it cannot be written."""
+    entity: ModbusEntityDescription = valid_entity_description
+    entity = entity.__class__(
+        **{
+            **entity.__dict__,
+            "conv_shift_bits": 4,
+            "conv_sum_scale": [1.0, 0.1],
+            "control_type": ControlType.NUMBER,
+        }
+    )
+    with patch(
+        "custom_components.modbus_local_gateway.entity_management.base._LOGGER.warning"
+    ):
+        assert not entity.validate()
+
+
+def test_validate_unsigned_bitfield_accepted_when_writable(
+    valid_entity_description: ModbusEntityDescription,
+) -> None:
+    """The ordinary case still validates."""
+    entity: ModbusEntityDescription = valid_entity_description
+    entity = entity.__class__(
+        **{
+            **entity.__dict__,
+            "conv_bits": 1,
+            "conv_shift_bits": 4,
+            "control_type": ControlType.SWITCH,
+        }
+    )
+    assert entity.validate()
diff --git a/tests/test_modbus_device_info.py b/tests/test_modbus_device_info.py
index 3b4a0ea..9c4c99b 100644
--- a/tests/test_modbus_device_info.py
+++ b/tests/test_modbus_device_info.py
@@ -32,14 +32,15 @@ def test_entity_load() -> None:
     name: Read-Write Entity
     address: 1
 
-  entity_invalid_bits:
-    name: Entity Invalid Bits
+  entity_bitfield_switch:
+    name: Entity Bitfield Switch
     address: 2
     control: switch
     bits: 1
+    shift_bits: 4
 
-  entity_invalid_shift:
-    name: Entity Invalid Shift
+  entity_shifted_switch:
+    name: Entity Shifted Switch
     address: 3
     control: switch
     shift_bits: 1
@@ -78,9 +79,17 @@ def test_entity_load() -> None:
         device = ModbusDeviceInfo("test.yaml")
 
         entities = device.entity_descriptions
-        assert len(entities) == 5
+        assert len(entities) == 7
         assert device.model == "Model"
         assert device.manufacturer == "Manufacturer"
+        # A switch on a bit field is valid: writing it is a read-modify-write.
+        assert any(
+            e.key == "entity_bitfield_switch"
+            and e.conv_bits == 1
+            and e.conv_shift_bits == 4
+            for e in entities
+        )
+        assert any(e.key == "entity_shifted_switch" for e in entities)
         assert any(
             e.key == "entity_rw" and e.data_type == ModbusDataType.HOLDING_REGISTER
             for e in entities

From f051a6fd115936c76ec7dcbfae25a745568ab789 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Prus-Zaj=C4=85czkowski?=
 <bartlomiej.prus@brainly.com>
Date: Mon, 17 Aug 2026 16:40:51 +0200
Subject: [PATCH 2/4] Correct and tighten the docstrings

Fixes three claims that were wrong or unresolvable, and cuts the reasoning that
served the author rather than the reader.

merge_into_registers said "Modbus cannot write individual bits of a holding
register". FC 0x16, Mask Write Register, does exactly that - pymodbus exposes it
as mask_write_register. The read-modify-write is still the right choice, but for
a different reason: FC 0x16 is optional and cannot span a multi-register field.
Says that instead, in both places the claim appeared.

_validate_bitfield said the merge "assumes an unsigned value and a single
register span". The span half is untrue - test_merge_across_two_registers proves
multi-register merges work and the validator never checks register_count - so it
described a constraint that does not exist. It also called the gate "the
writable control types" when ControlType.TEXT is writable and not in the tuple;
the three are now named.

field_geometry described its behaviour with `bits` / `shift_bits`, which are
device-YAML keys, while the function takes `desc`. It now names the attributes
it reads and maps them to the keys once.

_read_current_registers argued from register_count being at most 4 against a
default chunk of 8. Both correct, but that is why the decision was made rather
than anything a caller needs, and it buried the two facts that matter: hold the
lock, and a failed read abandons the write.

merge_into_registers also explained that _swap_registers is its own inverse -
a question about the code, not the contract. Moved to the call site.

Two test docstrings: one described the case in "zone1"/"zone2" terms that appear
nowhere else in the repository, the other called bits 12-19 of a 32-bit entity
"the 32-bit boundary" when they straddle the boundary between the two registers.
---
 .../modbus_local_gateway/conversion.py        | 23 ++++++++++--------
 .../entity_management/base.py                 |  7 +++---
 .../modbus_local_gateway/tcp_client.py        | 24 ++++++++-----------
 tests/test_conversion.py                      |  4 ++--
 4 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/custom_components/modbus_local_gateway/conversion.py b/custom_components/modbus_local_gateway/conversion.py
index 7446f08..c8651ac 100644
--- a/custom_components/modbus_local_gateway/conversion.py
+++ b/custom_components/modbus_local_gateway/conversion.py
@@ -212,10 +212,12 @@ def _descale(self, num: float, desc: ModbusEntityDescription) -> int:
         return int(round(num))
 
     def field_geometry(self, desc: ModbusEntityDescription) -> tuple[int, int]:
-        """Return (shift, mask) for a bit field.
+        """Return (shift, mask) for the bit field described by `desc`.
 
-        When `bits` is omitted the field is taken to run from `shift_bits` up to
-        the top of the register span, which matches how the read path behaves.
+        The geometry comes from `desc.conv_shift_bits` and `desc.conv_bits` -
+        the `shift_bits` and `bits` keys of a device YAML. With no `conv_bits`
+        the field runs from the shift to the top of the register span, which is
+        how the read path already treats it.
         """
         shift: int = desc.conv_shift_bits or 0
         width: int = desc.conv_bits or (16 * (desc.register_count or 1) - shift)
@@ -246,19 +248,20 @@ def merge_into_registers(
     ) -> list[int]:
         """Merge a bit-field value into the register(s) currently on the device.
 
-        Modbus cannot write individual bits of a holding register, so writing a
-        field declared with `bits` / `shift_bits` means reading the whole
-        register, replacing just that field, and writing it back. The caller is
-        responsible for doing the read and the write under the client lock so
-        the pair is atomic.
+        Writing a field declared with `bits` / `shift_bits` means reading the
+        whole register, replacing that field and writing it back. FC 0x16 (Mask
+        Write Register) would do this on the device, but it is optional and
+        cannot span a multi-register field.
 
-        `_swap_registers` is its own inverse for every supported swap type, so
-        the same call un-swaps on the way in and re-swaps on the way out.
+        The caller must hold the client lock across the read and the write.
         """
         if desc.conv_sum_scale:
             raise NotSupportedError("Setting of scaled sums is not supported")
 
         data_type = self._get_number_data_type(desc)
+        # Same _swap_registers call is used here and on the return: it is its own
+        # inverse, so it un-swaps into native order here and re-swaps back into
+        # device order below.
         raw = self.client.convert_from_registers(
             self._swap_registers(current_registers, desc), data_type=data_type
         )
diff --git a/custom_components/modbus_local_gateway/entity_management/base.py b/custom_components/modbus_local_gateway/entity_management/base.py
index 7041e65..e44c0b2 100644
--- a/custom_components/modbus_local_gateway/entity_management/base.py
+++ b/custom_components/modbus_local_gateway/entity_management/base.py
@@ -108,11 +108,10 @@ def validate(self) -> bool:
     def _validate_bitfield(self) -> bool:
         """Check constraints for writable bit fields.
 
-        Writing a `bits` / `shift_bits` field is a read-modify-write, which
-        merges the field into the register currently on the device. That
-        arithmetic assumes an unsigned value and a single register span.
+        The merge assumes an unsigned value: a signed field has no well-defined
+        representation once masked into part of a register.
 
-        Deliberately limited to the writable control types: applying it to
+        Limited to the number, switch and select controls - applying it to
         sensors would stop already-working entities from being created.
         """
         if not (self.conv_bits or self.conv_shift_bits):
diff --git a/custom_components/modbus_local_gateway/tcp_client.py b/custom_components/modbus_local_gateway/tcp_client.py
index 5f709d4..93c311b 100644
--- a/custom_components/modbus_local_gateway/tcp_client.py
+++ b/custom_components/modbus_local_gateway/tcp_client.py
@@ -216,16 +216,13 @@ async def _write_registers_individually(
     async def _read_current_registers(self, entity: ModbusContext) -> list[int]:
         """Read the register(s) backing a bit field, for a read-modify-write.
 
-        Must be called with the client lock held so the read and the following
-        write cannot be interleaved with a poll.
-
-        The whole span is read in a single transaction (`max_read_size` is the
-        field's own size) rather than honouring the device's `max_register_read`
-        chunking: a field split across two reads could tear if the device
-        changed in between. `register_count` is at most 4 and the default chunk
-        is 8, so this only differs for a gateway that cannot read the field in
-        one go - and there it fails loudly and aborts the write, which is the
-        safe outcome.
+        Must be called with the client lock held, so the read and the write it
+        feeds cannot be interleaved with a poll.
+
+        The span is read in one transaction rather than in `max_register_read`
+        chunks: a field split across two reads could tear if the device changed
+        in between. A failed read raises, abandoning the write - merging onto a
+        guess would clear the field's neighbours.
         """
         response: ModbusPDU | None = await self.read_data(
             func=self.read_holding_registers,
@@ -267,10 +264,9 @@ async def write_data(self, entity: ModbusContext, value: Any) -> ModbusPDU | Non
             if entity.desc.data_type == ModbusDataType.HOLDING_REGISTER:
                 conversion = Conversion(type(self))
                 if entity.desc.conv_bits or entity.desc.conv_shift_bits:
-                    # Modbus has no bit write for holding registers: read the
-                    # whole register, replace just this field, write it back.
-                    # Still inside `self.lock`, so no poll or other write can
-                    # land between the read and the write.
+                    # No dependable device-side bit write (FC 0x16 is optional):
+                    # read the register, replace this field, write it back. Still
+                    # inside `self.lock`, so nothing lands in between.
                     registers = conversion.merge_into_registers(
                         entity.desc,
                         value,
diff --git a/tests/test_conversion.py b/tests/test_conversion.py
index fccd4c4..a532629 100644
--- a/tests/test_conversion.py
+++ b/tests/test_conversion.py
@@ -717,7 +717,7 @@ def test_merge_single_bit(current: int, value: int, expected: int) -> None:
 
 
 def test_merge_low_byte_preserves_high_byte() -> None:
-    """The packed-zone case: writing zone1 must not zero zone2.
+    """Two 8-bit fields in one register: writing the low byte must not zero the high byte.
 
     A register holding 30 in the high byte and 6 in the low byte; writing 45
     to the low byte must leave the high byte at 30.
@@ -743,7 +743,7 @@ def test_merge_high_byte_preserves_low_byte() -> None:
 
 
 def test_merge_across_two_registers() -> None:
-    """A field spanning the 32-bit boundary of a two-register entity."""
+    """A field straddling the boundary between the two registers of a 32-bit entity."""
     conversion = Conversion(client=AsyncModbusTcpClient)
     desc = _number_desc(register_count=2, conv_bits=8, conv_shift_bits=12)
 

From 2df3845cc691ffc6826276b739d3034c8f2f67e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Prus-Zaj=C4=85czkowski?=
 <bartlomiej.prus@brainly.com>
Date: Tue, 25 Aug 2026 11:13:54 +0200
Subject: [PATCH 3/4] Document writable bit fields in the README

`bits` and `shift_bits` were listed only among the read-side math operations.
On a writable entity they now drive a read-modify-write, which is what lets
several independent controls share one register, so the README needs to say so -
including the two options that are rejected there and why.

Lists all three control types the validator accepts: number, select and switch.
Omitting select would have made the neighbouring `signed` / `sum_scale`
restriction look inapplicable to it, when it applies.
---
 README.md | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/README.md b/README.md
index c477b05..afaaa0f 100644
--- a/README.md
+++ b/README.md
@@ -166,6 +166,27 @@ For all entity definitions:
     - E.g., `sum_scale: [1, 10000]` for two registers starting at `address: 5` uses r1=5, r2=6, calculating r1 * 1 + r2 * 10000.
   - `shift_bits`: Bit shift right (integer).
   - `bits`: Bit mask length (integer).
+    - On a **writable** entity (`control: number`, `control: select` or `control: switch`),
+      `bits` and `shift_bits` make the entity address a *bit field*: the write becomes a
+      read-modify-write, so the other bits of the register keep their values. The read and the
+      write are issued under the client lock, so a poll cannot interleave between them.
+    - This lets several independent controls share one register. E.g. two switches in register 0,
+      one on bit 1 and one on bit 2, where toggling either leaves the other untouched:
+      ```yaml
+      heating:
+        address: 0
+        bits: 1
+        shift_bits: 1
+        control: switch
+      hot_water:
+        address: 0
+        bits: 1
+        shift_bits: 2
+        control: switch
+      ```
+    - `signed` and `sum_scale` are rejected on a *writable* bit field — neither has a meaningful
+      inverse when merging a value back into part of a register. They remain valid on read-only
+      entities.
   - `multiplier`: Scaling factor (float).
   - `offset`: Adds an offset (float).
 - **Display**:

From 29cb11368a0bf04b190c5c98c8bf5ba785cb4280 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Prus-Zaj=C4=85czkowski?=
 <bartlomiej.prus@brainly.com>
Date: Tue, 25 Aug 2026 18:36:33 +0200
Subject: [PATCH 4/4] Validate bit-field geometry and reject the options on
 coils

Two gaps found in review, both letting a bad config load and fail later.

A writable bit field was never checked against the registers it names. With
register_count 1 and shift_bits 17, field_geometry computes a width of -1 and
the FIRST WRITE raises "ValueError: negative shift count" from 1 << width - so
the entity loads fine and breaks when someone presses the switch. Now rejected
at load with the other bit-field constraints: negative shift, non-positive
width, or shift + width past the end of the span.

That also means an explicit `bits: 0` is now distinguished from an omitted
`bits`, in the validator and in field_geometry, and rejected. It was being read
as "the whole register", which is not a plausible reading of what the author
meant.

Coils are the second gap. A read_write_boolean switch carrying bits or
shift_bits passed validation, but write_data takes the COIL branch and ignores
both - so the config silently lacked the documented behaviour. A coil is already
a single bit, so the options are rejected there.

The geometry checks live in their own method: adding them inline pushed
_validate_bitfield past pylint's return-statement limit, and they are a
separable question from the signed/sum_scale ones.
---
 .../modbus_local_gateway/conversion.py        |  3 +-
 .../entity_management/base.py                 | 34 +++++++++-
 tests/test_entity_management_base.py          | 65 ++++++++++++++++++-
 3 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/custom_components/modbus_local_gateway/conversion.py b/custom_components/modbus_local_gateway/conversion.py
index c8651ac..4d43b03 100644
--- a/custom_components/modbus_local_gateway/conversion.py
+++ b/custom_components/modbus_local_gateway/conversion.py
@@ -219,8 +219,9 @@ def field_geometry(self, desc: ModbusEntityDescription) -> tuple[int, int]:
         the field runs from the shift to the top of the register span, which is
         how the read path already treats it.
         """
+        span: int = 16 * (desc.register_count or 1)
         shift: int = desc.conv_shift_bits or 0
-        width: int = desc.conv_bits or (16 * (desc.register_count or 1) - shift)
+        width: int = desc.conv_bits if desc.conv_bits is not None else span - shift
         return shift, (1 << width) - 1
 
     def _convert_from_decimal(
diff --git a/custom_components/modbus_local_gateway/entity_management/base.py b/custom_components/modbus_local_gateway/entity_management/base.py
index e44c0b2..c09ebe4 100644
--- a/custom_components/modbus_local_gateway/entity_management/base.py
+++ b/custom_components/modbus_local_gateway/entity_management/base.py
@@ -111,10 +111,13 @@ def _validate_bitfield(self) -> bool:
         The merge assumes an unsigned value: a signed field has no well-defined
         representation once masked into part of a register.
 
+        The geometry must also fit the register span, and a coil is already a
+        single bit so the options mean nothing there.
+
         Limited to the number, switch and select controls - applying it to
         sensors would stop already-working entities from being created.
         """
-        if not (self.conv_bits or self.conv_shift_bits):
+        if self.conv_bits is None and self.conv_shift_bits is None:
             return True
         if self.control_type not in (
             ControlType.NUMBER,
@@ -143,6 +146,35 @@ def _validate_bitfield(self) -> bool:
                 CONV_SHIFT_BITS,
             )
             return False
+        return self._validate_bitfield_geometry()
+
+    def _validate_bitfield_geometry(self) -> bool:
+        """The field must be a real run of bits inside the registers it names."""
+        if self.data_type == ModbusDataType.COIL:
+            _LOGGER.warning(
+                "Unable to create entity for %s: %s and %s have no meaning on a "
+                "coil, which is already a single bit",
+                self.key,
+                CONV_BITS,
+                CONV_SHIFT_BITS,
+            )
+            return False
+
+        span: int = 16 * (self.register_count or 1)
+        shift: int = self.conv_shift_bits or 0
+        width: int = self.conv_bits if self.conv_bits is not None else span - shift
+        if shift < 0 or width <= 0 or shift + width > span:
+            _LOGGER.warning(
+                "Unable to create entity for %s: %s %s / %s %s does not fit the "
+                "%s bits it addresses",
+                self.key,
+                CONV_SHIFT_BITS,
+                shift,
+                CONV_BITS,
+                width,
+                span,
+            )
+            return False
         return True
 
     def _validate_scan_interval(self) -> bool:
diff --git a/tests/test_entity_management_base.py b/tests/test_entity_management_base.py
index ebe2878..b1acf91 100644
--- a/tests/test_entity_management_base.py
+++ b/tests/test_entity_management_base.py
@@ -9,7 +9,10 @@
 from custom_components.modbus_local_gateway.entity_management.base import (
     ModbusEntityDescription,
 )
-from custom_components.modbus_local_gateway.entity_management.const import ControlType
+from custom_components.modbus_local_gateway.entity_management.const import (
+    ControlType,
+    ModbusDataType,
+)
 
 
 def test_validate_both_float_and_string(
@@ -183,3 +186,63 @@ def test_validate_unsigned_bitfield_accepted_when_writable(
         }
     )
     assert entity.validate()
+
+
+def _writable_bitfield(
+    entity: ModbusEntityDescription, **overrides
+) -> ModbusEntityDescription:
+    """Build a writable bit-field description from the valid fixture."""
+    return entity.__class__(
+        **{
+            **entity.__dict__,
+            "control_type": ControlType.SWITCH,
+            **overrides,
+        }
+    )
+
+
+@pytest.mark.parametrize(
+    ("overrides", "reason"),
+    [
+        (
+            {"register_count": 1, "conv_shift_bits": 17},
+            "shift past the end of the span gives a negative width, and the "
+            "first write would raise from 1 << width",
+        ),
+        (
+            {"register_count": 1, "conv_bits": 8, "conv_shift_bits": 12},
+            "field starts inside the span but runs off the end",
+        ),
+        (
+            {"conv_bits": 0, "conv_shift_bits": 0},
+            "an explicit bits: 0 is a mistake, not a request for the whole register",
+        ),
+        (
+            {"data_type": ModbusDataType.COIL, "conv_bits": 1, "conv_shift_bits": 4},
+            "a coil is already one bit and the write path ignores both options",
+        ),
+    ],
+    ids=["shift_past_span", "width_overflows_span", "explicit_zero_width", "coil"],
+)
+def test_validate_bitfield_geometry_rejected(
+    valid_entity_description: ModbusEntityDescription,
+    overrides: dict,
+    reason: str,
+) -> None:
+    """Bad geometry is refused at load, rather than failing at the first write."""
+    entity = _writable_bitfield(valid_entity_description, **overrides)
+    with patch(
+        "custom_components.modbus_local_gateway.entity_management.base._LOGGER.warning"
+    ) as mock_warning:
+        assert not entity.validate(), reason
+        mock_warning.assert_called_once()
+
+
+def test_validate_bitfield_spanning_two_registers_accepted(
+    valid_entity_description: ModbusEntityDescription,
+) -> None:
+    """The geometry check must not reject a field that legitimately spans registers."""
+    entity = _writable_bitfield(
+        valid_entity_description, register_count=2, conv_bits=8, conv_shift_bits=12
+    )
+    assert entity.validate()