From dd568f9ef4720f73cfeafa295aa5ab9e1dd8d56c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20R=C3=BChl?= Date: Sat, 3 Jun 2023 11:17:45 +0200 Subject: [PATCH] fix(plc4go): fixed some quality issues --- plc4go/examples/ads/discovery/Discovery.go | 2 +- .../hello_world_plc4go_bacnet_discovery.go | 2 +- .../hello_world_plc4go_knx_discovery.go | 2 +- plc4go/internal/ads/Discoverer.go | 4 +++- .../bacnetip/BACnetVirtualLinkLayerService.go | 8 +++++-- plc4go/internal/bacnetip/Connection.go | 23 +++++++++++-------- plc4go/internal/bacnetip/IOCBModule.go | 14 +++++------ plc4go/internal/bacnetip/MessageCodec.go | 12 ++++++++-- plc4go/internal/bacnetip/PDU.go | 4 ++-- .../bacnetip/UDPCommunicationsModule.go | 6 ++++- plc4go/internal/knxnetip/Browser.go | 12 +++++----- plc4go/pkg/api/cache/plcConnectionLease.go | 8 +++---- plc4go/spi/default/DefaultCodec.go | 23 +++++++++++-------- .../spi/model/DefaultPlcBrowseRequest_test.go | 4 ++-- .../DefaultPlcSubscriptionRequest_test.go | 2 +- .../model/DefaultPlcUnsubscriptionRequest.go | 6 ++--- .../transports/utils/TransportLogger_test.go | 2 +- plc4go/tools/plc4xgenerator/gen.go | 2 +- 18 files changed, 81 insertions(+), 55 deletions(-) diff --git a/plc4go/examples/ads/discovery/Discovery.go b/plc4go/examples/ads/discovery/Discovery.go index 354bfabf533..e19cc19680b 100644 --- a/plc4go/examples/ads/discovery/Discovery.go +++ b/plc4go/examples/ads/discovery/Discovery.go @@ -29,7 +29,7 @@ import ( func main() { discoverer := ads.NewDiscoverer() - discoverer.Discover(context.Background(), func(event apiModel.PlcDiscoveryItem) { + _ = discoverer.Discover(context.Background(), func(event apiModel.PlcDiscoveryItem) { print(event) }) time.Sleep(time.Second * 5) diff --git a/plc4go/examples/bacnet/discovery/hello_world_plc4go_bacnet_discovery.go b/plc4go/examples/bacnet/discovery/hello_world_plc4go_bacnet_discovery.go index b115eaa277e..0201c29766d 100644 --- a/plc4go/examples/bacnet/discovery/hello_world_plc4go_bacnet_discovery.go +++ b/plc4go/examples/bacnet/discovery/hello_world_plc4go_bacnet_discovery.go @@ -78,6 +78,6 @@ func main() { } log.Info().Str("connection string", connStr).Msg("Connected") connection := connectionResult.GetConnection() - defer connection.BlockingClose() + connection.BlockingClose() } } diff --git a/plc4go/examples/knx/discovery/hello_world_plc4go_knx_discovery.go b/plc4go/examples/knx/discovery/hello_world_plc4go_knx_discovery.go index a97862b3d3f..6ccb6732e1c 100644 --- a/plc4go/examples/knx/discovery/hello_world_plc4go_knx_discovery.go +++ b/plc4go/examples/knx/discovery/hello_world_plc4go_knx_discovery.go @@ -68,7 +68,7 @@ func main() { } log.Info().Str("connection string", connStr).Msg("Connected") connection := connectionResult.GetConnection() - defer connection.BlockingClose() + connection.BlockingClose() // Try to find all KNX devices on the current network browseRequest, err := connection.BrowseRequestBuilder(). diff --git a/plc4go/internal/ads/Discoverer.go b/plc4go/internal/ads/Discoverer.go index 34fdc419cf7..89fb53e01ef 100644 --- a/plc4go/internal/ads/Discoverer.go +++ b/plc4go/internal/ads/Discoverer.go @@ -237,7 +237,9 @@ func (d *Discoverer) Discover(ctx context.Context, callback func(event apiModel. defer func() { for _, discoveryItem := range discoveryItems { if discoveryItem.socket != nil { - discoveryItem.socket.Close() + if err := discoveryItem.socket.Close(); err != nil { + d.log.Debug().Err(err).Msg("errored") + } } } }() diff --git a/plc4go/internal/bacnetip/BACnetVirtualLinkLayerService.go b/plc4go/internal/bacnetip/BACnetVirtualLinkLayerService.go index edacc4a8abf..b5c2f3d9b5d 100644 --- a/plc4go/internal/bacnetip/BACnetVirtualLinkLayerService.go +++ b/plc4go/internal/bacnetip/BACnetVirtualLinkLayerService.go @@ -165,9 +165,13 @@ func (m *UDPMultiplexer) Close() error { log.Debug().Msg("Close") // pass along the close to the director(s) - m.directPort.Close() + if err := m.directPort.Close(); err != nil { + log.Debug().Err(err).Msg("errored") + } if m.broadcastPort != nil { - m.broadcastPort.Close() + if err := m.broadcastPort.Close(); err != nil { + log.Debug().Err(err).Msg("errored") + } } return nil } diff --git a/plc4go/internal/bacnetip/Connection.go b/plc4go/internal/bacnetip/Connection.go index 5c19e558d9e..6bb7a865e0c 100644 --- a/plc4go/internal/bacnetip/Connection.go +++ b/plc4go/internal/bacnetip/Connection.go @@ -101,15 +101,7 @@ func (c *Connection) ConnectWithContext(ctx context.Context) <-chan plc4go.PlcCo }() for c.IsConnected() { c.log.Trace().Msg("Polling data") - incomingMessageChannel := c.messageCodec.GetDefaultIncomingMessageChannel() - timeout := time.NewTimer(20 * time.Millisecond) - defer utils.CleanupTimer(timeout) - select { - case message := <-incomingMessageChannel: - // TODO: implement mapping to subscribers - log.Info().Msgf("Received \n%v", message) - case <-timeout.C: - } + c.passToDefaultIncomingMessageChannel() } c.log.Info().Msg("Ending incoming message transfer") }() @@ -118,6 +110,19 @@ func (c *Connection) ConnectWithContext(ctx context.Context) <-chan plc4go.PlcCo return ch } +func (c *Connection) passToDefaultIncomingMessageChannel() { + incomingMessageChannel := c.messageCodec.GetDefaultIncomingMessageChannel() + timeout := time.NewTimer(20 * time.Millisecond) + defer utils.CleanupTimer(timeout) + select { + case message := <-incomingMessageChannel: + // TODO: implement mapping to subscribers + log.Info().Msgf("Received \n%v", message) + case <-timeout.C: + log.Info().Msg("Message was not handled") + } +} + func (c *Connection) GetConnection() plc4go.PlcConnection { return c } diff --git a/plc4go/internal/bacnetip/IOCBModule.go b/plc4go/internal/bacnetip/IOCBModule.go index ae6a65de7fa..904b1a5217e 100644 --- a/plc4go/internal/bacnetip/IOCBModule.go +++ b/plc4go/internal/bacnetip/IOCBModule.go @@ -299,17 +299,17 @@ type PriorityItem struct { // A PriorityQueue implements heap.Interface and holds Items. type PriorityQueue []*PriorityItem -func (pq PriorityQueue) Len() int { return len(pq) } +func (pq *PriorityQueue) Len() int { return len(*pq) } -func (pq PriorityQueue) Less(i, j int) bool { +func (pq *PriorityQueue) Less(i, j int) bool { // We want Pop to give us the highest, not lowest, priority so we use greater than here. - return pq[i].priority > pq[j].priority + return (*pq)[i].priority > (*pq)[j].priority } -func (pq PriorityQueue) Swap(i, j int) { - pq[i], pq[j] = pq[j], pq[i] - pq[i].index = i - pq[j].index = j +func (pq *PriorityQueue) Swap(i, j int) { + (*pq)[i], (*pq)[j] = (*pq)[j], (*pq)[i] + (*pq)[i].index = i + (*pq)[j].index = j } func (pq *PriorityQueue) Push(x any) { diff --git a/plc4go/internal/bacnetip/MessageCodec.go b/plc4go/internal/bacnetip/MessageCodec.go index bbe1240af71..128a17e4ee6 100644 --- a/plc4go/internal/bacnetip/MessageCodec.go +++ b/plc4go/internal/bacnetip/MessageCodec.go @@ -112,7 +112,11 @@ func (m *ApplicationLayerMessageCodec) Send(message spi.Message) error { return errors.Wrap(err, "error creating IOCB") } go func() { - go m.bipSimpleApplication.RequestIO(iocb) + go func() { + if err := m.bipSimpleApplication.RequestIO(iocb); err != nil { + log.Debug().Err(err).Msg("errored") + } + }() iocb.Wait() if iocb.ioError != nil { // TODO: handle error @@ -142,7 +146,11 @@ func (m *ApplicationLayerMessageCodec) SendRequest(ctx context.Context, message return errors.Wrap(err, "error creating IOCB") } go func() { - go m.bipSimpleApplication.RequestIO(iocb) + go func() { + if err := m.bipSimpleApplication.RequestIO(iocb); err != nil { + + } + }() iocb.Wait() if err := iocb.ioError; err != nil { if err := handleError(err); err != nil { diff --git a/plc4go/internal/bacnetip/PDU.go b/plc4go/internal/bacnetip/PDU.go index 8308e45a3d1..c0e2c04d08d 100644 --- a/plc4go/internal/bacnetip/PDU.go +++ b/plc4go/internal/bacnetip/PDU.go @@ -209,7 +209,7 @@ func (a *Address) decodeAddress(addr any) error { if m { log.Debug().Msg("combined pattern") groups := combined_pattern.FindStringSubmatch(addr) - net := groups[0] + _net := groups[0] global_broadcast := groups[1] local_broadcast := groups[2] local_addr := groups[3] @@ -223,7 +223,7 @@ func (a *Address) decodeAddress(addr any) error { a := func(...any) { } - a(net, global_broadcast, local_broadcast, local_addr, local_ip_addr, local_ip_net, local_ip_port, route_addr, route_ip_addr, route_ip_port) + a(_net, global_broadcast, local_broadcast, local_addr, local_ip_addr, local_ip_net, local_ip_port, route_addr, route_ip_addr, route_ip_port) } panic("parsing not yet ported") case AddressTuple[string, uint16]: diff --git a/plc4go/internal/bacnetip/UDPCommunicationsModule.go b/plc4go/internal/bacnetip/UDPCommunicationsModule.go index 01a3de195db..07e8fc56129 100644 --- a/plc4go/internal/bacnetip/UDPCommunicationsModule.go +++ b/plc4go/internal/bacnetip/UDPCommunicationsModule.go @@ -277,7 +277,11 @@ func (d *UDPDirector) handleRead() { } pdu := NewPDU(bvlc, WithPDUSource(saddr), WithPDUDestination(daddr)) // send the PDU up to the client - go d._response(pdu) + go func() { + if err := d._response(pdu); err != nil { + log.Debug().Err(err).Msg("errored") + } + }() } func (d *UDPDirector) handleError(err error) { diff --git a/plc4go/internal/knxnetip/Browser.go b/plc4go/internal/knxnetip/Browser.go index 585e8807bc0..8898ffcae19 100644 --- a/plc4go/internal/knxnetip/Browser.go +++ b/plc4go/internal/knxnetip/Browser.go @@ -554,10 +554,10 @@ func (m Browser) calculateAddresses(query DeviceQuery) ([]driverModel.KnxAddress } func (m Browser) explodeSegment(segment string, min uint8, max uint8) ([]uint8, error) { - var options []uint8 + var segmentOptions []uint8 if strings.Contains(segment, "*") { for i := min; i <= max; i++ { - options = append(options, i) + segmentOptions = append(segmentOptions, i) } } else if strings.HasPrefix(segment, "[") && strings.HasSuffix(segment, "]") { segment = strings.TrimPrefix(segment, "[") @@ -574,14 +574,14 @@ func (m Browser) explodeSegment(segment string, min uint8, max uint8) ([]uint8, return nil, err } for i := localMin; i <= localMax; i++ { - options = append(options, uint8(i)) + segmentOptions = append(segmentOptions, uint8(i)) } } else { option, err := strconv.ParseUint(segment, 10, 8) if err != nil { return nil, err } - options = append(options, uint8(option)) + segmentOptions = append(segmentOptions, uint8(option)) } } } else { @@ -590,10 +590,10 @@ func (m Browser) explodeSegment(segment string, min uint8, max uint8) ([]uint8, return nil, err } if uint8(value) >= min && uint8(value) <= max { - options = append(options, uint8(value)) + segmentOptions = append(segmentOptions, uint8(value)) } } - return options, nil + return segmentOptions, nil } func (m Browser) parseAssociationTable(deviceDescriptor uint16, knxGroupAddresses []driverModel.KnxGroupAddress, value values.PlcValue) (driverModel.KnxGroupAddress, uint16) { diff --git a/plc4go/pkg/api/cache/plcConnectionLease.go b/plc4go/pkg/api/cache/plcConnectionLease.go index 41f6b0a69a9..34c2b6817b3 100644 --- a/plc4go/pkg/api/cache/plcConnectionLease.go +++ b/plc4go/pkg/api/cache/plcConnectionLease.go @@ -121,13 +121,13 @@ func (t *plcConnectionLease) Close() <-chan plc4go.PlcConnectionCloseResult { // Extract the trace entries from the connection. var traces []tracer.TraceEntry if t.IsTraceEnabled() { - tracer := t.GetTracer() + _tracer := t.GetTracer() // Save all traces. - traces = tracer.GetTraces() + traces = _tracer.GetTraces() // Clear the log. - tracer.ResetTraces() + _tracer.ResetTraces() // Reset the connection id back to the one without the lease-id. - tracer.SetConnectionId(t.connection.GetConnectionId()) + _tracer.SetConnectionId(t.connection.GetConnectionId()) } // Return the connection to the connection container and don't actually close it. diff --git a/plc4go/spi/default/DefaultCodec.go b/plc4go/spi/default/DefaultCodec.go index 2eb58690ede..ddf2819a03a 100644 --- a/plc4go/spi/default/DefaultCodec.go +++ b/plc4go/spi/default/DefaultCodec.go @@ -328,9 +328,8 @@ mainLoop: if m.customMessageHandling(codec, message) { workerLog.Trace().Msg("Custom handling handled the message") continue mainLoop - } else { - workerLog.Trace().Msg("Custom handling didn't handle the message") } + workerLog.Trace().Msg("Custom handling didn't handle the message") } workerLog.Trace().Msg("Handle message") @@ -340,14 +339,18 @@ mainLoop: // If the message has not been handled and a default handler is provided, call this ... if !messageHandled { workerLog.Trace().Msg("Message was not handled") - timeout := time.NewTimer(time.Millisecond * 40) - defer utils.CleanupTimer(timeout) - select { - case m.defaultIncomingMessageChannel <- message: - case <-timeout.C: - timeout.Stop() - workerLog.Warn().Msgf("Message discarded\n%s", message) - } + m.passToDefaultIncomingMessageChannel(workerLog, message) } } } + +func (m *defaultCodec) passToDefaultIncomingMessageChannel(workerLog zerolog.Logger, message spi.Message) { + timeout := time.NewTimer(time.Millisecond * 40) + defer utils.CleanupTimer(timeout) + select { + case m.defaultIncomingMessageChannel <- message: + case <-timeout.C: + timeout.Stop() + workerLog.Warn().Msgf("Message discarded\n%s", message) + } +} diff --git a/plc4go/spi/model/DefaultPlcBrowseRequest_test.go b/plc4go/spi/model/DefaultPlcBrowseRequest_test.go index d7a9fe2a55e..4b0f30ef14e 100644 --- a/plc4go/spi/model/DefaultPlcBrowseRequest_test.go +++ b/plc4go/spi/model/DefaultPlcBrowseRequest_test.go @@ -265,7 +265,7 @@ func TestDefaultPlcBrowseRequest_ExecuteWithInterceptor(t *testing.T) { queryNames: tt.fields.queryNames, queries: tt.fields.queries, } - assert.Equalf(t, tt.want, d.ExecuteWithInterceptor(tt.args.interceptor), "ExecuteWithInterceptor(%v)", tt.args.interceptor) + assert.Equalf(t, tt.want, d.ExecuteWithInterceptor(tt.args.interceptor), "ExecuteWithInterceptor(func(%t))", tt.args.interceptor != nil) }) } } @@ -306,7 +306,7 @@ func TestDefaultPlcBrowseRequest_ExecuteWithInterceptorWithContext(t *testing.T) queryNames: tt.fields.queryNames, queries: tt.fields.queries, } - assert.Equalf(t, tt.want, d.ExecuteWithInterceptorWithContext(tt.args.ctx, tt.args.interceptor), "ExecuteWithInterceptorWithContext(%v, %v)", tt.args.ctx, tt.args.interceptor) + assert.Equalf(t, tt.want, d.ExecuteWithInterceptorWithContext(tt.args.ctx, tt.args.interceptor), "ExecuteWithInterceptorWithContext(%v, func(%t))", tt.args.ctx, tt.args.interceptor != nil) }) } } diff --git a/plc4go/spi/model/DefaultPlcSubscriptionRequest_test.go b/plc4go/spi/model/DefaultPlcSubscriptionRequest_test.go index 66a3556e20c..20283420a72 100644 --- a/plc4go/spi/model/DefaultPlcSubscriptionRequest_test.go +++ b/plc4go/spi/model/DefaultPlcSubscriptionRequest_test.go @@ -436,7 +436,7 @@ func TestDefaultPlcSubscriptionRequestBuilder_AddPreRegisteredConsumer(t *testin intervals: tt.fields.intervals, preRegisteredConsumers: tt.fields.preRegisteredConsumers, } - assert.Equalf(t, tt.want, d.AddPreRegisteredConsumer(tt.args.name, tt.args.consumer), "AddPreRegisteredConsumer(%v, %v)", tt.args.name, tt.args.consumer) + assert.Equalf(t, tt.want, d.AddPreRegisteredConsumer(tt.args.name, tt.args.consumer), "AddPreRegisteredConsumer(%v, func(%t))", tt.args.name, tt.args.consumer != nil) }) } } diff --git a/plc4go/spi/model/DefaultPlcUnsubscriptionRequest.go b/plc4go/spi/model/DefaultPlcUnsubscriptionRequest.go index 56407bc0226..97c564c13a4 100644 --- a/plc4go/spi/model/DefaultPlcUnsubscriptionRequest.go +++ b/plc4go/spi/model/DefaultPlcUnsubscriptionRequest.go @@ -35,16 +35,16 @@ func NewDefaultPlcUnsubscriptionRequest() *DefaultPlcUnsubscriptionRequest { return &DefaultPlcUnsubscriptionRequest{} } -func (d DefaultPlcUnsubscriptionRequest) Execute() <-chan apiModel.PlcUnsubscriptionRequestResult { +func (d *DefaultPlcUnsubscriptionRequest) Execute() <-chan apiModel.PlcUnsubscriptionRequestResult { //TODO implement me panic("implement me") } -func (d DefaultPlcUnsubscriptionRequest) ExecuteWithContext(ctx context.Context) <-chan apiModel.PlcUnsubscriptionRequestResult { +func (d *DefaultPlcUnsubscriptionRequest) ExecuteWithContext(ctx context.Context) <-chan apiModel.PlcUnsubscriptionRequestResult { //TODO implement me panic("implement me") } -func (d DefaultPlcUnsubscriptionRequest) IsAPlcMessage() bool { +func (d *DefaultPlcUnsubscriptionRequest) IsAPlcMessage() bool { return true } diff --git a/plc4go/spi/transports/utils/TransportLogger_test.go b/plc4go/spi/transports/utils/TransportLogger_test.go index 35a023aa3b3..08cd5a5c4cc 100644 --- a/plc4go/spi/transports/utils/TransportLogger_test.go +++ b/plc4go/spi/transports/utils/TransportLogger_test.go @@ -159,7 +159,7 @@ func TestWithLogger(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if got := WithLogger(tt.args.log); !assert.Equal(t, tt.want, got) { - t.Errorf("WithLogger() = %v, want %v", got, tt.want) + t.Errorf("WithLogger() = func(%t), want (%t)", got != nil, tt.want != nil) } }) } diff --git a/plc4go/tools/plc4xgenerator/gen.go b/plc4go/tools/plc4xgenerator/gen.go index b7eedec04ec..0591b0d2b93 100644 --- a/plc4go/tools/plc4xgenerator/gen.go +++ b/plc4go/tools/plc4xgenerator/gen.go @@ -138,7 +138,7 @@ type Generator struct { } func (g *Generator) Printf(format string, args ...any) { - fmt.Fprintf(&g.buf, format, args...) + _, _ = fmt.Fprintf(&g.buf, format, args...) } // File holds a single parsed file and associated data.