feat: removed deprecated identifiers.

Also support warpped errors in ToExceptionCode, and fixed some lint warnings

.golangci.yml

@@ -55,7 +55,12 @@ issues:
       linters:
         - maintidx
         - unused
-
+
+    - path: "(.+)_test.go"
+      linters:
+        - govet
+      text: "declaration of \"t\" shadows declaration"
+
   exclude:
     - "G104: Errors unhandled." # turn off errcheck
     - "declaration of \"[a-z]\" shadows declaration at .*_test.go" # ignore shadowing a single character variables in tests

README.md

@@ -1,4 +1,4 @@
-# ModbusOne [![GoDoc](https://godoc.org/github.com/xiegeo/modbusone?status.svg)](https://godoc.org/github.com/xiegeo/modbusone)
+# ModbusOne [![Go Reference](https://pkg.go.dev/badge/github.com/xiegeo/modbusone?utm_source=godoc#section-documentation.svg)](https://pkg.go.dev/github.com/xiegeo/modbusone?utm_source=godoc#section-documentation)
 A Modbus library for Go, with unified client and server APIs.
 One implementation to rule them all.
 <details>
@@ -14,13 +14,13 @@ func handlerGenerator(name string) modbusone.ProtocolHandler {
             fmt.Printf("%v ReadHoldingRegisters from %v, quantity %v\n",
                 name, address, quantity)
             r := make([]uint16, quantity)
-            // application code that fills in r here
+            // Application code that fills in r here.
             return r, nil
         },
         WriteHoldingRegisters: func(address uint16, values []uint16) error {
             fmt.Printf("%v WriteHoldingRegisters from %v, quantity %v\n",
                 name, address, len(values))
-            // application code here
+            // Application code here.
             return nil
         },
         OnErrorImp: func(req modbusone.PDU, errRep modbusone.PDU) {
@@ -29,7 +29,7 @@ func handlerGenerator(name string) modbusone.ProtocolHandler {
     }
 }
 
-// serial is a fake serial port
+// serial is a fake serial port.
 type serial struct {
     io.ReadCloser
     io.WriteCloser
@@ -241,6 +241,17 @@ Development tools:
 
 ## Breaking Changes
 
+2022-09-09 v1.0.0
+
+V1 released with the following depreciated identifiers removed:
+
+- The `Server` interface is removed, use `ServerCloser` instead.
+- Public global variable `DebugOut` is changed to private. Use `SetDebugOut(w io.Writer)` instead for thread safety.
+- Type alias `type ProtocalHandler = ProtocolHandler` removed.
+- Function redirect from `GetRTUBidirectionSizeFromHeader` to `GetRTUBidirectionalSizeFromHeader` removed.
+- Function redirect from `NewRTUCLient` to `NewRTUClient` removed.
+
+
 2018-09-27 v0.2.0
 
 - NewRTUPacketReader returns PacketReader interface instead of io.Reader. When a new RTU server or client receives a SerialContext, it will test if it is also a PacketReader, and only create a new NewRTUPacketReader if not.

data.go

@@ -62,16 +62,20 @@ func BoolsToData(values []bool, fc FunctionCode) ([]byte, error) {
 
 	for v := 0; v < count; v++ {
 		if values[v] {
+			// set bit in byte to true
 			byteVal |= 1 << bitNr
 		}
-		if bitNr == 7 {
+		switch {
+		case bitNr == 7:
+			// last bit in byte, set data value and go to the next byte
 			data[byteNr] = byteVal
 			byteVal = 0
 			bitNr = 0
 			byteNr++
-		} else if v+1 == count {
+		case v+1 == count:
+			// last bit, set byte value and exit loop
 			data[byteNr] = byteVal
-		} else {
+		default:
 			bitNr++
 		}
 	}

examples/memory/client.go

@@ -7,9 +7,8 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/xiegeo/modbusone"
-
 	"github.com/fatih/color"
+	"github.com/xiegeo/modbusone"
 )
 
 func runClient(c *modbusone.RTUClient) {

examples/memory/main.go

@@ -71,7 +71,7 @@ func main() {
 	if *fillData == "am3" {
 		fillAm3()
 	}
-	var device modbusone.Server
+	var device modbusone.ServerCloser
 	if *isClient {
 		if *writeSizeLimit > modbusone.MaxRTUSize || *readSizeLimit > modbusone.MaxRTUSize {
 			fmt.Fprintf(os.Stderr, "write/read size limit is too big")

examples_test.go

@@ -20,13 +20,13 @@ func handlerGenerator(name string) modbusone.ProtocolHandler {
 			fmt.Printf("%v ReadHoldingRegisters from %v, quantity %v\n",
 				name, address, quantity)
 			r := make([]uint16, quantity)
-			// application code that fills in r here
+			// Application code that fills in r here.
 			return r, nil
 		},
 		WriteHoldingRegisters: func(address uint16, values []uint16) error {
 			fmt.Printf("%v WriteHoldingRegisters from %v, quantity %v\n",
 				name, address, len(values))
-			// application code here
+			// Application code here.
 			return nil
 		},
 		OnErrorImp: func(req modbusone.PDU, errRep modbusone.PDU) {
@@ -35,7 +35,7 @@ func handlerGenerator(name string) modbusone.ProtocolHandler {
 	}
 }
 
-// serial is a fake serial port
+// serial is a fake serial port.
 type serial struct {
 	io.ReadCloser
 	io.WriteCloser

failover_rtu_client.go

@@ -3,6 +3,7 @@ package modbusone
 import (
 	"bytes"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"sync/atomic"
 	"time"
@@ -18,8 +19,8 @@ type FailoverRTUClient struct {
 	actions              chan rtuAction
 }
 
-// FailoverRTUClient is also a Server.
-var _ Server = &FailoverRTUClient{}
+// FailoverRTUClient is also a ServerCloser.
+var _ ServerCloser = &FailoverRTUClient{}
 
 // NewFailoverRTUClient create a new client with failover function communicating over SerialContext with the
 // give slaveID as default.
@@ -199,7 +200,7 @@ func (c *FailoverRTUClient) Serve(handler ProtocolHandler) error {
 				}
 				rp, err := react.data.GetPDU()
 				if err != nil {
-					if err == ErrorCrc {
+					if errors.Is(err, ErrorCrc) {
 						atomic.AddInt64(&c.com.Stats().CrcErrors, 1)
 					} else {
 						atomic.AddInt64(&c.com.Stats().OtherErrors, 1)

handler2serial_test.go

@@ -65,7 +65,7 @@ func (s *mockSerial) BytesDelay(n int) time.Duration { return 0 }
 func (s *mockSerial) Stats() *Stats                  { return &s.s }
 
 // TestHandler runs through each of simplymodbus.ca's samples, conforms both
-// end-to-end behavior and wire format
+// end-to-end behavior and wire format.
 func TestHandler(t *testing.T) {
 	// DebugOut = os.Stdout
 	slaveID := byte(0x11)

modbus.go

@@ -5,27 +5,17 @@
 package modbusone
 
 import (
+	"errors"
 	"fmt"
 	"io"
 )
 
-// Server is a common interface for all Clients and Servers that use ProtocolHandlers.
-// Deprecated: added Closer in ServerCloser.
-type Server interface {
-	Serve(handler ProtocolHandler) error
-}
-
 // ServerCloser is the common interface for all Clients and Servers that use ProtocolHandlers.
 type ServerCloser interface {
-	Server
+	Serve(handler ProtocolHandler) error
 	io.Closer
 }
 
-// ProtocalHandler is a misspelling of ProtocolHandler.
-//
-// Deprecated: misspelling.
-type ProtocalHandler = ProtocolHandler
-
 // ProtocolHandler handles PDUs based on if it is a write or read from the local
 // perspective.
 type ProtocolHandler interface {
@@ -257,11 +247,12 @@ func ToExceptionCode(err error) ExceptionCode {
 	if err == nil {
 		debugf("ToExceptionCode: unexpected covert nil error to ExceptionCode")
 	}
-	e, ok := err.(ExceptionCode)
+	var e ExceptionCode
+	ok := errors.As(err, &e)
 	if ok {
 		return e
 	}
-	if err == ErrFcNotSupported {
+	if errors.Is(err, ErrFcNotSupported) {
 		return EcIllegalFunction
 	}
 	return EcServerDeviceFailure

modbus_test.go

@@ -0,0 +1,30 @@
+package modbusone
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"testing"
+)
+
+func TestToExceptionCode(t *testing.T) {
+	tests := []struct {
+		err  error
+		want ExceptionCode
+	}{
+		{err: EcIllegalDataAddress, want: EcIllegalDataAddress},
+		{err: ErrFcNotSupported, want: EcIllegalFunction},
+		{err: errors.New("other errors"), want: EcServerDeviceFailure},
+	}
+	for _, tt := range tests {
+		t.Run(tt.err.Error(), func(t *testing.T) {
+			if got := ToExceptionCode(tt.err); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ToExceptionCode() = %v, want %v", got, tt.want)
+			}
+			errWarped := fmt.Errorf("warped error for %w", tt.err)
+			if got := ToExceptionCode(errWarped); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ToExceptionCode() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

packet_reader.go

@@ -145,13 +145,6 @@ func GetRTUSizeFromHeader(header []byte, isClient bool) int {
 	return GetPDUSizeFromHeader(header[1:], isClient) + 3
 }
 
-// GetRTUBidirectionSizeFromHeader is a misspelling of GetRTUBidirectionalSizeFromHeader.
-//
-// Deprecated: misspelling.
-func GetRTUBidirectionSizeFromHeader(header []byte) int {
-	return GetRTUBidirectionalSizeFromHeader(header)
-}
-
 // GetRTUBidirectionalSizeFromHeader is like GetRTUSizeFromHeader, except for any direction
 // by checking the CRC for disambiguation of length.
 func GetRTUBidirectionalSizeFromHeader(header []byte) int {

rtu_client.go

@@ -25,16 +25,9 @@ type Client interface {
 	DoTransaction(req PDU) error
 }
 
-// RTUClient is a Client.
+// Asserts that RTUClient implements Client.
 var _ Client = &RTUClient{}
 
-// NewRTUCLient is an miscapitalization of NewRTUClient.
-//
-// Deprecated: miscapitalization.
-func NewRTUCLient(com SerialContext, slaveID byte) *RTUClient {
-	return NewRTUClient(com, slaveID)
-}
-
 // NewRTUClient create a new client communicating over SerialContext with the
 // given slaveID as default.
 func NewRTUClient(com SerialContext, slaveID byte) *RTUClient {
@@ -182,7 +175,7 @@ func (c *RTUClient) Serve(handler ProtocolHandler) error {
 				}
 				rp, err := react.data.GetPDU()
 				if err != nil {
-					if err == ErrorCrc {
+					if errors.Is(err, ErrorCrc) {
 						atomic.AddInt64(&c.com.Stats().CrcErrors, 1)
 					} else {
 						atomic.AddInt64(&c.com.Stats().OtherErrors, 1)

rtu_server.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"math/rand"
 	"runtime"
-	"sync"
 	"sync/atomic"
 	"time"
 )
@@ -74,7 +73,7 @@ func (s *RTUServer) Serve(handler ProtocolHandler) error {
 		var err error
 		p, err = r.GetPDU()
 		if err != nil {
-			if err == ErrorCrc {
+			if errors.Is(err, ErrorCrc) {
 				atomic.AddInt64(&s.com.Stats().CrcErrors, 1)
 			} else {
 				atomic.AddInt64(&s.com.Stats().OtherErrors, 1)
@@ -139,35 +138,31 @@ func Uint64ToSlaveID(n uint64) (byte, error) {
 	return byte(n), nil
 }
 
-// DebugOut sets where to print debug messages.
-//
-// Deprecated: please use SetDebugOut instead.
-var DebugOut io.Writer
-var debugLock sync.Mutex
+// debugOutput sets where to print debug messages.
+var debugOutput atomic.Value
 
 // SetDebugOut to print debug messages, set to nil to turn off debug output.
 func SetDebugOut(w io.Writer) {
-	// todo: refactor to use pkg/sync/atomic/#Value once DebugOut is removed.
-	debugLock.Lock()
-	defer debugLock.Unlock()
-	DebugOut = w
+	debugOutput.Store(&w) // nil can not be store to atomic.Value directly, but a pointer to nil can.
 }
 
 const monkey = false
 
 func debugf(format string, a ...interface{}) {
-	if monkey && rand.Float32() < 0.5 { //nolint:gosec
+	if monkey && rand.Float32() < 0.5 { //nolint:gosec // Monkey testing's random doesn't need secure random numbers.
 		runtime.Gosched()
 	}
-	debugLock.Lock()
-	defer debugLock.Unlock()
-	if DebugOut == nil {
+	debugWriterP, _ := debugOutput.Load().(*io.Writer)
+	if debugWriterP == nil || *debugWriterP == nil {
+		// SetDebugOut is never called, or last set to nil
 		return
 	}
-	fmt.Fprintf(DebugOut, "[%s]", time.Now().Format("06-01-02 15:04:05.000000"))
-	fmt.Fprintf(DebugOut, format, a...)
+	debugWriter := *debugWriterP
+	fmt.Fprintf(debugWriter, "[%s]", time.Now().Format("06-01-02 15:04:05.000000"))
+	fmt.Fprintf(debugWriter, format, a...)
 	lf := len(format)
 	if lf > 0 && format[lf-1] != '\n' {
-		fmt.Fprintln(DebugOut)
+		// make sure each call to debugf ends in new line.
+		fmt.Fprintln(debugWriter)
 	}
 }

tcp_client.go

@@ -16,14 +16,14 @@ type TCPClient struct {
 	cancle        context.CancelFunc
 	conn          io.ReadWriteCloser
 	SlaveID       byte
-	_handler      ProtocalHandler // very private, always use getHandler
+	_handler      ProtocolHandler // very private, always use getHandler
 	_handlerReady sync.WaitGroup
 	exitError     error // set this before call to cancel
 	locker        sync.Mutex
 }
 
-// TCPClient is also a Server.
-var _ Server = &TCPClient{}
+// TCPClient is also a ServerCloser.
+var _ ServerCloser = &TCPClient{}
 
 // NewTCPClient create a new client communicating over a TCP connection with the
 // given slaveID as default.
@@ -56,7 +56,7 @@ func (c *TCPClient) getHandler() ProtocolHandler {
 	return c._handler
 }
 
-// Close closes the client and closes the TCP connection
+// Close closes the client and closes the TCP connection.
 func (c *TCPClient) Close() error {
 	if c.exitError == nil {
 		c.exitError = errors.New("closed by user action")

tcp_server.go

@@ -120,7 +120,7 @@ func (s *TCPServer) Serve(handler ProtocolHandler) error {
 	}
 }
 
-// Close closes the server and closes the listener
+// Close closes the server and closes the listener.
 func (s *TCPServer) Close() error {
 	return s.listener.Close()
 }