SDO fixes & improvements (#21)

* Changed : renamed to NewNodeProcessor for consistency with type * Refactored : continue changes to remove logrus Changed : removed MainCallback for now * Refactored : add slog to fileobject * Changed : removed logrus from http gateway & added slog Changed : minor coherence refactor for naming * Fixed : error in gateway example, should now work + slog * Changed : removed logrus from CAN drivers, replaced by slog * Changed : removed logrus from SDO client * Fixed : forgot to add logger to virtual can * Removed : logrus in commented server test http * Changed : bus manager doesn't use logrus Changed : .gitignore remove created file during testing Changed : removed last references to logrus & run log tidy * Removed : no more timerNext ==> unused Changed : regrouped TPDOs, RPDOs in one function call * Added : new SDO function calls for local node (ReadUint8,...ReadInt8,...ReadFloat32) Added : od decode to exact type Changed : new functions ReadAny,ReadAnyExact and WriteAny which replace Read & Write. Some further simplifications in the functions * Tests : added a test for big block sdo download. This test should fail because of a bug in server * Fixed : write out buffer at the end of each sub-block, this fixes issue where we would not empty out buffer in case of fast transfers Fixed : remove raw casting as server could panic, particularly if an unexpected error occurs during io.Copy * Fixed : increase size of buffer to avoid never emptying buffer on block transfer write * Fixed : block transfer test was wrong because used expedited and not block transfer. Added a big & small block download * Fixed : also fix write / read block and rename test * Format : improve formatting * Tests : adding parametrized tests to improve test coverage * Fixed : clear extra error info on new SDO transfer * Fixed : issue in block upload retransmit : an incorrect amount of data would get CRC not calculated, causing issues in CRC Fixed : issue in the number of data read because of wrong usage of bytes.Available() Changed : readObjectDictionary takes a countExact argument to read exactly a given amount of bytes from OD * Refactored : renamed 'server' to s for simplicity & consistency * Fixed : sdo block test should work properly, also increased speed by changing client processing period * Fixed : failing TPDO test because using ReadUint8 instead of ReadInt8 --------- Co-authored-by: Samuel Lee <[email protected]>
samsamfire · Jan 16, 2025 · 7a3e913 · 7a3e913
1 parent b08c01b
commit 7a3e913
Show file tree

Hide file tree

Showing 20 changed files with 578 additions and 331 deletions.
diff --git a/pkg/network/client_test.go b/pkg/network/client_test.go
@@ -52,7 +52,7 @@ func BenchmarkSDOReadLocal(b *testing.B) {
 	b.StartTimer()
 	var value uint64
 	for i := 0; i < b.N; i++ {
-		value, err = client.ReadUint64(0x55, 0x201B, 0x0)
+		value, err = client.ReadUint(0x201B, 0x0)
 	}
 	result = value
 }
@@ -65,32 +65,53 @@ func TestSDOReadBlock(t *testing.T) {
 
 }
 
-func TestSDOWriteBlock(t *testing.T) {
+func TestClientBlock(t *testing.T) {
 	network := CreateNetworkTest()
+	network2 := CreateNetworkEmptyTest()
 	defer network.Disconnect()
-	data := []byte("some random string some random string some random string some random string some random string some random string some random string")
+	defer network2.Disconnect()
 	node := network.controllers[NodeIdTest].GetNode()
 	file, err := os.CreateTemp("", "filename")
 	assert.Nil(t, err)
 	node.GetOD().AddFile(0x3333, "File entry", file.Name(), os.O_RDWR|os.O_CREATE, os.O_RDWR|os.O_CREATE)
 	assert.Nil(t, err)
-	err = network.WriteRaw(NodeIdTest, 0x3333, 0, data, false)
-	assert.Nil(t, err)
-}
 
-func TestSDOReadWriteBlock(t *testing.T) {
-	network := CreateNetworkTest()
-	defer network.Disconnect()
-	data := []byte("some random string some random string some random string some random string some random string some random string some random string")
-	node, err := network.Local(NodeIdTest)
-	assert.Nil(t, err)
-	file, err := os.CreateTemp("", "filename")
-	assert.Nil(t, err)
-	node.GetOD().AddFile(0x3333, "File entry", file.Name(), os.O_RDWR|os.O_CREATE, os.O_RDWR|os.O_CREATE)
-	assert.Nil(t, err)
-	err = network.WriteRaw(NodeIdTest, 0x3333, 0, data, false)
-	assert.Nil(t, err)
-	data2, err := network.ReadAll(NodeIdTest, 0x3333, 0)
-	assert.Nil(t, err)
-	assert.Equal(t, data, data2)
+	t.Run("small block", func(t *testing.T) {
+		data := []byte(`some random string some random string some random
+		string some random string some random 
+		string some random string some random string`)
+		w, err := network2.SDOClient.NewRawWriter(NodeIdTest, 0x3333, 0, true, 0)
+		assert.Nil(t, err)
+		n, err := w.Write(data)
+		assert.Nil(t, err)
+		assert.EqualValues(t, len(data), n)
+	})
+
+	t.Run("big block", func(t *testing.T) {
+		data := make([]byte, 10_000)
+		network2.SDOClient.SetProcessingPeriod(100)
+		w, err := network2.SDOClient.NewRawWriter(NodeIdTest, 0x3333, 0, true, 10_000)
+
+		assert.Nil(t, err)
+		n, err := w.Write(data)
+		assert.Nil(t, err)
+		assert.EqualValues(t, 10_000, n)
+	})
+	t.Run("write file and read back", func(t *testing.T) {
+		file, err := os.CreateTemp("", "filename")
+		assert.Nil(t, err)
+		node.GetOD().AddFile(0x3333, "File entry", file.Name(), os.O_RDWR|os.O_CREATE, os.O_RDWR|os.O_CREATE)
+		assert.Nil(t, err)
+		data := []byte(`some random string some random string some random
+		string some random string some random 
+		string some random string some random string`)
+		w, err := network2.SDOClient.NewRawWriter(NodeIdTest, 0x3333, 0, true, 0)
+		assert.Nil(t, err)
+		n, err := w.Write(data)
+		assert.Nil(t, err)
+		assert.EqualValues(t, len(data), n)
+		data2, err := network.ReadAll(NodeIdTest, 0x3333, 0)
+		assert.Nil(t, err)
+		assert.Equal(t, data, data2)
+	})
 }
diff --git a/pkg/network/node_test.go b/pkg/network/node_test.go
@@ -56,15 +56,15 @@ func TestReadLocal(t *testing.T) {
 	assert.Nil(t, err)
 	t.Run("Read any", func(t *testing.T) {
 		for indexName, key := range SDO_UNSIGNED_READ_MAP {
-			val, _ := local.Read(indexName, "")
+			val, _ := local.ReadAny(indexName, "")
 			assert.Equal(t, key, val)
 		}
 		for indexName, key := range SDO_INTEGER_READ_MAP {
-			val, _ := local.Read(indexName, "")
+			val, _ := local.ReadAny(indexName, "")
 			assert.Equal(t, key, val)
 		}
 		for indexName, key := range SDO_FLOAT_READ_MAP {
-			val, _ := local.Read(indexName, "")
+			val, _ := local.ReadAny(indexName, "")
 			assert.InDelta(t, key, val, 1e-5)
 		}
 
@@ -117,7 +117,54 @@ func TestReadLocal(t *testing.T) {
 		val, _ := local.ReadFloat("REAL32 value", "")
 		assert.InDelta(t, 1500.1, val, 0.01)
 	})
+}
 
+func TestReadLocalExact(t *testing.T) {
+	network := CreateNetworkTest()
+	defer network.Disconnect()
+	local, _ := network.Local(NodeIdTest)
+	t.Run("uint8", func(t *testing.T) {
+		local.WriteAny("UNSIGNED8 value", 0, uint8(55))
+		v, err := local.ReadUint8("UNSIGNED8 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, uint8(55), v)
+	})
+	t.Run("uint16", func(t *testing.T) {
+		local.WriteAny("UNSIGNED16 value", 0, uint16(1234))
+		v, err := local.ReadUint16("UNSIGNED16 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, uint16(1234), v)
+	})
+	t.Run("uint32", func(t *testing.T) {
+		local.WriteAny("UNSIGNED32 value", 0, uint32(567899))
+		v, err := local.ReadUint32("UNSIGNED32 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, uint32(567899), v)
+	})
+	t.Run("int8", func(t *testing.T) {
+		local.WriteAny("INTEGER8 value", 0, int8(11))
+		v, err := local.ReadInt8("INTEGER8 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, int8(11), v)
+	})
+	t.Run("int16", func(t *testing.T) {
+		local.WriteAny("INTEGER16 value", 0, int16(11231))
+		v, err := local.ReadInt16("INTEGER16 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, int16(11231), v)
+	})
+	t.Run("int32", func(t *testing.T) {
+		local.WriteAny("INTEGER32 value", 0, int32(98789))
+		v, err := local.ReadInt32("INTEGER32 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, int32(98789), v)
+	})
+	t.Run("float32", func(t *testing.T) {
+		local.WriteAny("REAL32 value", 0, float32(0.6))
+		v, err := local.ReadFloat32("REAL32 value", 0)
+		assert.Nil(t, err)
+		assert.Equal(t, float32(0.6), v)
+	})
 }
 
 func TestReadRemote(t *testing.T) {

diff --git a/pkg/network/pdo_test.go b/pkg/network/pdo_test.go
@@ -46,12 +46,16 @@ func TestTpdo(t *testing.T) {
 		// received on other side
 		err = local.Write(0x2002, 0, int8(10))
 		assert.Nil(t, err)
-		time.Sleep(400 * time.Millisecond)
-
-		// Internal value received via PDO
-		val, err := remote.ReadUint8(0, 0x2002, 0)
-		assert.Nil(t, err)
-		assert.EqualValues(t, 10, val)
 
+		assert.Eventually(t,
+			func() bool {
+				// Internal value received via PDO
+				val, err := remote.ReadInt8(0x2002, 0)
+				if val == 10 && err == nil {
+					return true
+				}
+				return false
+			}, 5*time.Second, 100*time.Millisecond,
+		)
 	})
 }
diff --git a/pkg/node/controller.go b/pkg/node/controller.go
@@ -41,9 +41,8 @@ func (c *NodeProcessor) background(ctx context.Context) {
 			ticker.Stop()
 			return
 		case <-ticker.C:
-			syncWas := c.node.ProcessSYNC(PeriodUs, nil)
-			c.node.ProcessTPDO(syncWas, PeriodUs, nil)
-			c.node.ProcessRPDO(syncWas, PeriodUs, nil)
+			syncWas := c.node.ProcessSYNC(PeriodUs)
+			c.node.ProcessPDO(syncWas, PeriodUs)
 		}
 	}
 }

diff --git a/pkg/node/local.go b/pkg/node/local.go
@@ -37,34 +37,27 @@ type LocalNode struct {
 	TIME               *t.TIME
 }
 
-func (node *LocalNode) ProcessTPDO(syncWas bool, timeDifferenceUs uint32, timerNextUs *uint32) {
+func (node *LocalNode) ProcessPDO(syncWas bool, timeDifferenceUs uint32) {
 	if node.NodeIdUnconfigured {
 		return
 	}
-	nmtIsOperational := node.NMT.GetInternalState() == nmt.StateOperational
+	isOperational := node.NMT.GetInternalState() == nmt.StateOperational
 	for _, tpdo := range node.TPDOs {
-		tpdo.Process(timeDifferenceUs, timerNextUs, nmtIsOperational, syncWas)
+		tpdo.Process(timeDifferenceUs, isOperational, syncWas)
 	}
-}
-
-func (node *LocalNode) ProcessRPDO(syncWas bool, timeDifferenceUs uint32, timerNextUs *uint32) {
-	if node.NodeIdUnconfigured {
-		return
-	}
-	nmtIsOperational := node.NMT.GetInternalState() == nmt.StateOperational
 	for _, rpdo := range node.RPDOs {
-		rpdo.Process(timeDifferenceUs, timerNextUs, nmtIsOperational, syncWas)
+		rpdo.Process(timeDifferenceUs, isOperational, syncWas)
 	}
 }
 
-func (node *LocalNode) ProcessSYNC(timeDifferenceUs uint32, timerNextUs *uint32) bool {
+func (node *LocalNode) ProcessSYNC(timeDifferenceUs uint32) bool {
 	syncWas := false
 	sy := node.SYNC
 	if !node.NodeIdUnconfigured && sy != nil {
 
 		nmtState := node.NMT.GetInternalState()
 		nmtIsPreOrOperational := nmtState == nmt.StatePreOperational || nmtState == nmt.StateOperational
-		syncProcess := sy.Process(nmtIsPreOrOperational, timeDifferenceUs, timerNextUs)
+		syncProcess := sy.Process(nmtIsPreOrOperational, timeDifferenceUs)
 
 		switch syncProcess {
 		case s.EventRxOrTx:

diff --git a/pkg/node/node.go b/pkg/node/node.go
@@ -21,9 +21,8 @@ const (
 // A [Node] handles the CANopen stack.
 type Node interface {
 	// Cyclic tasks
-	ProcessTPDO(syncWas bool, timeDifferenceUs uint32, timerNextUs *uint32)
-	ProcessRPDO(syncWas bool, timeDifferenceUs uint32, timerNextUs *uint32)
-	ProcessSYNC(timeDifferenceUs uint32, timerNextUs *uint32) bool
+	ProcessPDO(syncWas bool, timeDifferenceUs uint32)
+	ProcessSYNC(timeDifferenceUs uint32) bool
 	ProcessMain(enableGateway bool, timeDifferenceUs uint32, timerNextUs *uint32) uint8
 	// Internal servers
 	Servers() []*sdo.SDOServer

diff --git a/pkg/node/remote.go b/pkg/node/remote.go
@@ -36,28 +36,22 @@ type RemoteNode struct {
 	emcy     *emergency.EMCY      // Emergency consumer (fake producer for logging internal errors)
 }
 
-func (node *RemoteNode) ProcessTPDO(syncWas bool, timeDifferenceUs uint32, timerNextUs *uint32) {
-
+func (node *RemoteNode) ProcessPDO(syncWas bool, timeDifferenceUs uint32) {
 	node.mu.Lock()
 	defer node.mu.Unlock()
+
 	for _, tpdo := range node.tpdos {
-		tpdo.Process(timeDifferenceUs, timerNextUs, true, syncWas)
+		tpdo.Process(timeDifferenceUs, true, syncWas)
 	}
-
-}
-
-func (node *RemoteNode) ProcessRPDO(syncWas bool, timeDifferenceUs uint32, timerNextUs *uint32) {
-	node.mu.Lock()
-	defer node.mu.Unlock()
 	for _, rpdo := range node.rpdos {
-		rpdo.Process(timeDifferenceUs, timerNextUs, true, syncWas)
+		rpdo.Process(timeDifferenceUs, true, syncWas)
 	}
 }
 
-func (node *RemoteNode) ProcessSYNC(timeDifferenceUs uint32, timerNextUs *uint32) bool {
+func (node *RemoteNode) ProcessSYNC(timeDifferenceUs uint32) bool {
 	syncWas := false
 	if node.sync != nil {
-		event := node.sync.Process(true, timeDifferenceUs, timerNextUs)
+		event := node.sync.Process(true, timeDifferenceUs)
 
 		switch event {
 		case sync.EventNone, sync.EventRxOrTx: