Repository: cocotb/cocotb

Issue:
Discussion: bool cast of weak drivers
![Image](https://github.com/user-attachments/assets/054a103a-9f4e-43cc-9b00-b82456b3b06f)

currently the 2.0 style Logic implementation only allows casts to boolean for 1/0 values.

W/L/H leads to a ValueError. For W this makes sense as it is essentially a 'weak X', but L and H are perfectly valid high and low values, usually encountered in pad logic or other scenarios involving Pulls or Open Drain scenarios (think I2C)

Hence the question: has it been discussed whether these should cast to a valid boolean?

Discussion:
@ktbarrett: I don't think people deal with weak values all that often and haven't heard anyone give any opinion on them until now.

What do you think should be done? Should L and H *always* be resolved to 0 and 1 in integer contexts, even when resolve_x is not set (ValueError)? Should there be a mode that resolves weak values first but then otherwise works like ValueError? If a new "resolve weak" mode is introduced, should it be default?

Also up for debate is my recent change here that removes L and H always being resolved. https://github.com/cocotb/cocotb/pull/4512/commits/3df14bfe588a513408238de5e181e5f61573eb51

This affects `__bool__`, `__int__`, `to_unsigned`, and `to_signed` IMO. @ktbarrett: I'm actually starting to like my "resolve weak" mode is default idea. There's weak and strong values in Verilog and we currently don't handle signal strength for Verilog values at all, so we are already implicitly resolving weak values there, so we should parallel that for VHDL.@markusdd: Upfront: I do not have a particulary strong opinion on it at all, I just stumbled over it when working on the deprecations in our codebase rgearding .integer and .to_unsigned(), so I started thinking about it and discussing with a colleague.

That being said:
weak 1s/0s are perfectly valid values, they just differ in behavior when something is driven over them. (e.g. a weak 1 will become a clean 0 when a strong 0 pulls it to ground instead of becoming X)
A 1 on an I2C bus or pullup on a pad is a fully qualified 1 in the digital sense. So thinking about it this way, a weak 1 or 0 should resolve to 1 or zero in a boolean sense, because that is what another real circuit would do as well. I think this - generally - would be expected behavior.

And as you say, in verilog this happens kind of implicitly right now anyway.

So I agree with your take: per default, boolean resolution of valid weak values (so excluding U and W which are weak X) should work and return the respective True or False values.

In rare cases where this is not intended having a switch to change that might be beneficial.


Overall it's true weak logic is used rarely, but certainly not 'never'. Especially Memory models like Flash or i2C sensor models often make use of them. Frankly, certain features like clock stall on i2c cannot even be simulated properly with weak values as everything would become X with strong drivers on all ends. And you also need it for pad pullup/down modes etc.

Long story short: I agree, resolving properly per default makes sense.

Solution patch:
diff --git a/docs/source/newsfragments/4551.change.1.rst b/docs/source/newsfragments/4551.change.1.rst
new file mode 100644
index 00000000..a01bf420
--- /dev/null
+++ b/docs/source/newsfragments/4551.change.1.rst
@@ -0,0 +1 @@
+:class:`bool(Logic) <cocotb.types.Logic>` casts now treat ``H`` as ``True`` and ``L`` as ``False`` instead of raising :exc:`ValueError`.
diff --git a/docs/source/newsfragments/4551.change.2.rst b/docs/source/newsfragments/4551.change.2.rst
new file mode 100644
index 00000000..56fdd3f2
--- /dev/null
+++ b/docs/source/newsfragments/4551.change.2.rst
@@ -0,0 +1 @@
+:meth:`.LogicArray.is_resolvable` now returns ``True`` for arrays containing ``H`` and ``L`` values.
diff --git a/docs/source/newsfragments/4551.change.rst b/docs/source/newsfragments/4551.change.rst
new file mode 100644
index 00000000..b703c021
--- /dev/null
+++ b/docs/source/newsfragments/4551.change.rst
@@ -0,0 +1 @@
+:class:`int(Logic) <cocotb.types.Logic>` casts now treat ``H`` as ``1`` and ``L`` as ``0`` instead of raising :exc:`ValueError`.
diff --git a/src/cocotb/types/logic.py b/src/cocotb/types/logic.py
index b654975e..7d2fbe71 100644
--- a/src/cocotb/types/logic.py
+++ b/src/cocotb/types/logic.py
@@ -87,7 +87,7 @@ class Logic:
 
     .. note::
 
-        The :class:`int` and :class:`bool` conversions will raise :exc:`ValueError` if the value is not ``0`` or ``1``.
+        The :class:`int` and :class:`bool` conversions will raise :exc:`ValueError` if the value is not ``0``, ``1``, ``L``, or ``H``.
 
     :class:`Logic` values are immutable and therefore hashable and can be placed in :class:`set`\ s and used as keys in :class:`dict`\ s.
 
@@ -229,16 +229,16 @@ class Logic:
         return ("U", "X", "0", "1", "Z", "W", "L", "H", "-")[self._repr]
 
     def __bool__(self) -> bool:
-        if self._repr == _0:
+        if self._repr in (_0, _L):
             return False
-        elif self._repr == _1:
+        elif self._repr in (_1, _H):
             return True
         raise ValueError(f"Cannot convert {self!r} to bool")
 
     def __int__(self) -> int:
-        if self._repr == _0:
+        if self._repr in (_0, _L):
             return 0
-        elif self._repr == _1:
+        elif self._repr in (_1, _H):
             return 1
         raise ValueError(f"Cannot convert {self!r} to int")
 
diff --git a/src/cocotb/types/logic_array.py b/src/cocotb/types/logic_array.py
index ee18b04d..2f42d116 100644
--- a/src/cocotb/types/logic_array.py
+++ b/src/cocotb/types/logic_array.py
@@ -325,6 +325,10 @@ class LogicArray(ArrayLike[Logic]):
         if self._value_as_int is None:
             # May convert list to str before converting to int.
             value_as_str = self._get_str()
+
+            # always resolve L and H to 0 and 1
+            value_as_str = value_as_str.translate(_resolve_lh_table)
+
             try:
                 self._value_as_int = int(value_as_str, 2)
             except ValueError:
@@ -332,9 +336,6 @@ class LogicArray(ArrayLike[Logic]):
                 if resolve is None:
                     resolve = RESOLVE_X
 
-                # resolve L and H to 0 and 1
-                value_as_str = value_as_str.translate(_resolve_lh_table)
-
                 # resolve remaining
                 resolve_table = _resolve_tables[resolve]
                 value_as_str = value_as_str.translate(resolve_table)
@@ -535,8 +536,10 @@ class LogicArray(ArrayLike[Logic]):
 
     @property
     def is_resolvable(self) -> bool:
-        """``True`` if all elements are ``0`` or ``1``."""
-        return all(bit in (Logic(0), Logic(1)) for bit in self)
+        """``True`` if all elements are ``0``, ``1``, ``L``, ``H``."""
+        return all(
+            bit in (Logic("0"), Logic("1"), Logic("L"), Logic("H")) for bit in self
+        )
 
     @property
     @deprecated("`.integer` property is deprecated. Use `value.to_unsigned()` instead.")
@@ -778,11 +781,9 @@ class LogicArray(ArrayLike[Logic]):
     def __invert__(self) -> "LogicArray":
         return LogicArray(~v for v in self)
 
+    @deprecated(
+        "`bool()` casts and using LogicArray in conditional expressions is deprecated. "
+        """Use explicit comparisons (e.g. `LogicArray() == "11"`) or `all`/`any` expressions instead."""
+    )
     def __bool__(self) -> bool:
-        warnings.warn(
-            "The behavior of bool casts and using LogicArray in conditionals may change in the future. "
-            """Use explicit comparisons (e.g. `LogicArray() == "11"`) instead""",
-            FutureWarning,
-            stacklevel=2,
-        )
         return any(v in (Logic("H"), Logic("1")) for v in self)


Test patch:
diff --git a/tests/pytest/test_logic.py b/tests/pytest/test_logic.py
index 159d6378..dcd0edd1 100644
--- a/tests/pytest/test_logic.py
+++ b/tests/pytest/test_logic.py
@@ -64,11 +64,19 @@ def test_logic_equality():
 
 def test_logic_bool_conversions():
     assert bool(Logic("1")) is True
+    assert bool(Logic("H")) is True
     assert bool(Logic("0")) is False
+    assert bool(Logic("L")) is False
     with pytest.raises(ValueError):
         bool(Logic("X"))
     with pytest.raises(ValueError):
         bool(Logic("Z"))
+    with pytest.raises(ValueError):
+        bool(Logic("U"))
+    with pytest.raises(ValueError):
+        bool(Logic("W"))
+    with pytest.raises(ValueError):
+        bool(Logic("-"))
 
 
 def test_logic_str_conversions():
@@ -88,10 +96,18 @@ def test_logic_index_cast():
 def test_logic_int_conversions():
     assert int(Logic("0")) == 0
     assert int(Logic("1")) == 1
+    assert int(Logic("L")) == 0
+    assert int(Logic("H")) == 1
     with pytest.raises(ValueError):
         int(Logic("X"))
     with pytest.raises(ValueError):
         int(Logic("Z"))
+    with pytest.raises(ValueError):
+        int(Logic("U"))
+    with pytest.raises(ValueError):
+        int(Logic("-"))
+    with pytest.raises(ValueError):
+        int(Logic("W"))
 
 
 def test_logic_repr():
diff --git a/tests/pytest/test_logic_array.py b/tests/pytest/test_logic_array.py
index a4379fe2..5c57be60 100644
--- a/tests/pytest/test_logic_array.py
+++ b/tests/pytest/test_logic_array.py
@@ -396,10 +396,10 @@ def test_null_vector():
 
 
 def test_bool_cast():
-    with pytest.warns(FutureWarning):
+    with pytest.warns(DeprecationWarning):
         assert LogicArray("0110")
     with warnings.catch_warnings():
-        warnings.filterwarnings(action="ignore", category=FutureWarning)
+        warnings.filterwarnings(action="ignore", category=DeprecationWarning)
         assert not LogicArray("0000")
         assert LogicArray("01XZ")
         assert LogicArray("XZ01")