diff --git a/util/modbus/connection.go b/util/modbus/connection.go index eadacafe9..c5b0c8d59 100644 --- a/util/modbus/connection.go +++ b/util/modbus/connection.go @@ -11,9 +11,9 @@ import ( type Connection struct { *logger meters.Connection - slaveID uint8 // duplicated from meters.Connection - logical meters.Logger - delay time.Duration + physical *meterConnection + slaveID uint8 // duplicated from meters.Connection + logical meters.Logger } func (c *Connection) Addr() string { @@ -24,8 +24,9 @@ func (c *Connection) Logger(logger meters.Logger) { c.logical = logger } +// Delay applies the delay to the shared physical connection func (c *Connection) Delay(delay time.Duration) { - c.delay = delay + c.physical.setDelay(delay) } func (c *Connection) Clone(slaveID uint8) *Connection { @@ -33,26 +34,23 @@ func (c *Connection) Clone(slaveID uint8) *Connection { slaveID: slaveID, Connection: c.Connection.Clone(slaveID), logger: c.logger, + physical: c.physical, } } -// TODO resolve conflicts +// ConnectDelay applies the connect delay to the shared physical connection func (c *Connection) ConnectDelay(delay time.Duration) { - if delay > 0 { - c.Connection.ConnectDelay(delay) - } + c.physical.setConnectDelay(delay) } -// TODO resolve conflicts +// Timeout applies the timeout to the shared physical connection func (c *Connection) Timeout(timeout time.Duration) { - if timeout > 0 { - _ = c.Connection.Timeout(timeout) - } + c.physical.setTimeout(timeout) } func (c *Connection) exec(fun func() ([]byte, error)) ([]byte, error) { return c.WithLogger(c.logical, func() ([]byte, error) { - time.Sleep(c.delay) + time.Sleep(c.physical.getDelay()) b, err := fun() if err != nil { diff --git a/util/modbus/modbus.go b/util/modbus/modbus.go index d8a6efaf8..c332d41f6 100644 --- a/util/modbus/modbus.go +++ b/util/modbus/modbus.go @@ -103,9 +103,52 @@ type meterConnection struct { meters.Connection proto Protocol refs int // count of references; first connection has ref count 0 + + // largest value requested by any of the sharing logical connections + delay time.Duration + connectDelay time.Duration + timeout time.Duration + *logger } +// setDelay applies the delay if larger than the current value +func (c *meterConnection) setDelay(delay time.Duration) { + mu.Lock() + defer mu.Unlock() + + c.delay = max(c.delay, delay) +} + +func (c *meterConnection) getDelay() time.Duration { + mu.Lock() + defer mu.Unlock() + + return c.delay +} + +// setConnectDelay applies the connect delay if larger than the current value +func (c *meterConnection) setConnectDelay(delay time.Duration) { + mu.Lock() + defer mu.Unlock() + + if delay > c.connectDelay { + c.connectDelay = delay + c.Connection.ConnectDelay(delay) + } +} + +// setTimeout applies the timeout if larger than the current value +func (c *meterConnection) setTimeout(timeout time.Duration) { + mu.Lock() + defer mu.Unlock() + + if timeout > c.timeout { + c.timeout = timeout + _ = c.Connection.Timeout(timeout) + } +} + var ( connections = make(map[string]*meterConnection) mu sync.Mutex @@ -176,6 +219,7 @@ func NewConnection(ctx context.Context, uri, device, comset string, baudrate int slaveID: slaveID, Connection: conn.Clone(slaveID), logger: conn.logger, + physical: conn, } return res, nil diff --git a/util/modbus/modbus_test.go b/util/modbus/modbus_test.go index ce2b3b614..7cfe597a5 100644 --- a/util/modbus/modbus_test.go +++ b/util/modbus/modbus_test.go @@ -2,10 +2,40 @@ package modbus import ( "testing" + "time" "github.com/stretchr/testify/require" ) +// TestSharedSettings ensures the largest delay and timeout wins for all +// connections sharing the same physical connection +func TestSharedSettings(t *testing.T) { + ctx := t.Context() + uri := "localhost:15020" + + c1, err := Settings{URI: uri, ID: 1, Delay: 2 * time.Second, Timeout: time.Second}.Connection(ctx) + require.NoError(t, err) + + c2, err := Settings{URI: uri, ID: 2, Delay: time.Second, Timeout: 3 * time.Second}.Connection(ctx) + require.NoError(t, err) + + // unset settings don't reset the shared values + c3, err := Settings{URI: uri, ID: 3}.Connection(ctx) + require.NoError(t, err) + + require.Same(t, c1.physical, c2.physical) + require.Same(t, c1.physical, c3.physical) + require.Same(t, c1.physical, c1.Clone(4).physical) + + for _, c := range []*Connection{c1, c2, c3} { + require.Equal(t, 2*time.Second, c.physical.getDelay()) + require.Equal(t, 3*time.Second, c.physical.timeout) + } + + // timeout has been applied to the physical connection + require.Equal(t, 3*time.Second, c1.physical.Connection.Timeout(3*time.Second)) +} + func TestParsePoint(t *testing.T) { tc := []struct { in string diff --git a/util/templates/template_modbus.go b/util/templates/template_modbus.go index 283106e28..09adcb2cf 100644 --- a/util/templates/template_modbus.go +++ b/util/templates/template_modbus.go @@ -69,9 +69,12 @@ func (t *Template) ModbusValues(renderMode int, values map[string]any) { typeParams := modbusConfig.Types[iface].Params for _, p := range typeParams { - // don't overwrite custom values - if values[p.Name] != nil { - continue + // don't overwrite custom values. Params the template deprecated in favour + // of the modbus definition are pre-populated with an empty string default. + if v := values[p.Name]; v != nil { + if s, ok := v.(string); !ok || s != "" { + continue + } } values[p.Name] = p.DefaultValue(renderMode)