From 3357fa13c6ec38f0c73a03f2f98d9a748c9635bc Mon Sep 17 00:00:00 2001
From: Andrew Nolte <anolte@hudson-trading.com>
Date: Tue, 23 Jan 2024 13:02:41 -0500
Subject: [PATCH] direct value access of vpi structs

---
 src/cocotb/share/lib/vpi/VpiCbHdl.cpp       |  3 +-
 src/cocotb/share/lib/vpi/VpiImpl.cpp        | 10 ++--
 tests/test_cases/test_struct/Makefile       | 19 ++++++++
 tests/test_cases/test_struct/test_struct.py | 53 +++++++++++++++++++++
 4 files changed, 78 insertions(+), 7 deletions(-)
 create mode 100644 tests/test_cases/test_struct/Makefile
 create mode 100644 tests/test_cases/test_struct/test_struct.py

diff --git a/src/cocotb/share/lib/vpi/VpiCbHdl.cpp b/src/cocotb/share/lib/vpi/VpiCbHdl.cpp
index a9dea7abb0..990f112807 100644
--- a/src/cocotb/share/lib/vpi/VpiCbHdl.cpp
+++ b/src/cocotb/share/lib/vpi/VpiCbHdl.cpp
@@ -243,7 +243,8 @@ int VpiSignalObjHdl::initialise(const std::string &name,
             m_indexable = false;  // Don't want to iterate over indices
             m_range_left = 0;
             m_range_right = m_num_elems - 1;
-        } else if (GpiObjHdl::get_type() == GPI_REGISTER ||
+        } else if (GpiObjHdl::get_type() == GPI_STRUCTURE ||
+                   GpiObjHdl::get_type() == GPI_REGISTER ||
                    GpiObjHdl::get_type() == GPI_NET) {
             vpiHandle hdl = GpiObjHdl::get_handle<vpiHandle>();
 
diff --git a/src/cocotb/share/lib/vpi/VpiImpl.cpp b/src/cocotb/share/lib/vpi/VpiImpl.cpp
index 11a6b5f37b..6138378298 100644
--- a/src/cocotb/share/lib/vpi/VpiImpl.cpp
+++ b/src/cocotb/share/lib/vpi/VpiImpl.cpp
@@ -210,6 +210,10 @@ GpiObjHdl *VpiImpl::create_gpi_obj_from_handle(vpiHandle new_hdl,
         case vpiStringVar:
         case vpiMemoryWord:
         case vpiInterconnectNet:
+        case vpiStructVar:
+        case vpiStructNet:
+        case vpiUnionVar:
+        case vpiUnionNet:
             new_obj =
                 new VpiSignalObjHdl(this, new_hdl, to_gpi_objtype(type), false);
             break;
@@ -229,12 +233,6 @@ GpiObjHdl *VpiImpl::create_gpi_obj_from_handle(vpiHandle new_hdl,
         case vpiInterconnectArray:
             new_obj = new VpiArrayObjHdl(this, new_hdl, to_gpi_objtype(type));
             break;
-        case vpiStructVar:
-        case vpiStructNet:
-        case vpiUnionVar:
-        case vpiUnionNet:
-            new_obj = new VpiObjHdl(this, new_hdl, to_gpi_objtype(type));
-            break;
         case vpiModule:
         case vpiInterface:
         case vpiModport:
diff --git a/tests/test_cases/test_struct/Makefile b/tests/test_cases/test_struct/Makefile
new file mode 100644
index 0000000000..4efc300423
--- /dev/null
+++ b/tests/test_cases/test_struct/Makefile
@@ -0,0 +1,19 @@
+# Copyright cocotb contributors
+# Licensed under the Revised BSD License, see LICENSE for details.
+# SPDX-License-Identifier: BSD-3-Clause
+
+
+TOPLEVEL_LANG ?= verilog
+
+ifneq ($(TOPLEVEL_LANG),verilog)
+
+all:
+	@echo "Skipping test due to TOPLEVEL_LANG=$(TOPLEVEL_LANG) not being verilog"
+clean::
+
+else
+
+include ../../designs/sample_module/Makefile
+MODULE:=test_struct
+
+endif
diff --git a/tests/test_cases/test_struct/test_struct.py b/tests/test_cases/test_struct/test_struct.py
new file mode 100644
index 0000000000..3601f14373
--- /dev/null
+++ b/tests/test_cases/test_struct/test_struct.py
@@ -0,0 +1,53 @@
+# Copyright cocotb contributors
+# Licensed under the Revised BSD License, see LICENSE for details.
+# SPDX-License-Identifier: BSD-3-Clause
+import cocotb
+from cocotb.triggers import Timer
+
+
+@cocotb.test(
+    expect_error=AttributeError
+    if cocotb.SIM_NAME.lower().startswith(("icarus", "ghdl", "riviera", "nvc"))
+    else (),
+    expect_fail=cocotb.LANGUAGE == "vhdl"
+    or cocotb.SIM_NAME.lower().startswith("modelsim"),
+)
+async def test_struct_direct(dut):
+    """Test getting and setting setting the value of an entire struct"""
+    # TODO: do this through handle.py
+    if cocotb.SIM_NAME.lower().startswith("verilator"):
+        print(dut.inout_if)  # LogicObject(sample_module.inout_if)
+        # print(dut.inout_if.a_in) # Doesn't exist
+        # print(dut.inout_if.b_out) # Doesn't exist
+
+        print("binstr: ", dut.inout_if._handle.get_signal_val_binstr())  # 00
+        assert dut.inout_if._handle.get_signal_val_binstr() == "00"
+    else:
+        print(dut.inout_if)  # HierarchyObject(sample_module.inout_if)
+        print(dut.inout_if.a_in)  # LogicObject(sample_module.inout_if.a_in)
+        print(dut.inout_if.b_out)  # LogicObject(sample_module.inout_if.b_out)
+
+        # seg fault if VpiObjHdl, works on VpiSignalObjHdl
+        # breakpoint()
+        print("binstr: ", dut.inout_if._handle.get_signal_val_binstr())  # xx
+        assert dut.inout_if._handle.get_signal_val_binstr() == "zz"
+
+        # test struct write -> individual signals
+        logic_obj = cocotb.handle.LogicObject(dut.inout_if._handle, dut.inout_if._path)
+        logic_obj._set_value(0, cocotb.scheduler._schedule_write)
+        await Timer(1000, "ns")
+        print("binstr: ", dut.inout_if._handle.get_signal_val_binstr())  # xx
+        assert dut.inout_if._handle.get_signal_val_binstr() == "00"
+        assert dut.inout_if.a_in.value == 0
+        assert dut.inout_if.b_out.value == 0
+
+        # test signal write -> struct value
+        dut.inout_if.a_in.value = 1
+        await Timer(1000, "ns")
+        print("binstr: ", dut.inout_if._handle.get_signal_val_binstr())
+        assert str(logic_obj.value.binstr) == "10"
+
+        dut.inout_if.b_out.value = 1
+        await Timer(1000, "ns")
+        print("binstr: ", dut.inout_if._handle.get_signal_val_binstr())
+        assert str(logic_obj.value.binstr) == "11"