add standard logging and linting

.github/workflows/test.yml

@@ -22,5 +22,11 @@ jobs:
     - name: Build
       run: go build -v ./...
 
+    - name: Lint
+      run: |
+        go run github.com/golangci/golangci-lint/cmd/golangci-lint run -v ./...
+        go get -u github.com/edaniels/golinters/cmd/combined
+        go vet -vettool=$(which combined) ./...
+
     - name: Test
       run: go test -v ./...

.golangci.yaml

@@ -0,0 +1,188 @@
+service:
+  # When updating this, also update bin/linters.sh accordingly
+  golangci-lint-version: 1.18.x # use the fixed version to not introduce new linters unexpectedly
+run:
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 600s
+  # which dirs to skip: they won't be analyzed;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but next dirs are always skipped independently
+  # from this option's value:
+  #     vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  skip-dirs:
+    - genfiles$
+    - vendor$
+    - test$
+
+  # which files to skip: they will be analyzed, but issues from them
+  # won't be reported. Default value is empty list, but there is
+  # no need to include all autogenerated files, we confidently recognize
+  # autogenerated files. If it's not please let us know.
+  skip-files:
+
+  tests: true
+
+linters:
+  enable-all: false
+  enable:
+      - staticcheck
+      - vet
+      - vetshadow
+      - goimports
+      - golint
+      - structcheck
+      - unconvert
+      - goconst
+      - interfacer
+      - misspell
+      - ineffassign
+      - varcheck
+      - deadcode
+      - gofmt
+      - unconvert
+      - ineffassign
+      - interfacer
+  disable:
+  fast: false
+
+linters-settings:
+  errcheck:
+  #   # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
+  #   # default is false: such cases aren't reported by default.
+    check-type-assertions: false
+
+  #   # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
+  #   # default is false: such cases aren't reported by default.
+    check-blank: true
+  govet:
+    # report about shadowed variables
+    check-shadowing: false
+  golint:
+    # minimal confidence for issues, default is 0.8
+    min-confidence: 0.3
+  gofmt:
+    # simplify code: gofmt with `-s` option, true by default
+    simplify: true
+  goimports:
+    # put imports beginning with prefix after 3rd-party packages;
+    # it's a comma-separated list of prefixes
+
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  misspell:
+    # Correct spellings using locale preferences for US or UK.
+    # Default is to use a neutral variety of English.
+    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
+    # locale: US
+  lll:
+    # max line length, lines longer will be reported. Default is 120.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
+    line-length: 140
+    # tab width in spaces. Default to 1.
+    tab-width: 1
+  unused:
+    # treat code as a program (not a library) and report unused exported identifiers; default is false.
+    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  unparam:
+    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
+    # and rta for programs with main packages. Default is cha.
+    algo: cha
+
+    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
+    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  gocritic:
+    enabled-checks:
+      - appendCombine
+      - argOrder
+      - assignOp
+      - badCond
+      - boolExprSimplify
+      - builtinShadow
+      - captLocal
+      - caseOrder
+      - codegenComment
+      - commentedOutCode
+      - commentedOutImport
+      - defaultCaseOrder
+      - deprecatedComment
+      - docStub
+      - dupArg
+      - dupBranchBody
+      - dupCase
+      - dupSubExpr
+      - elseif
+      - emptyFallthrough
+      - equalFold
+      - flagDeref
+      - flagName
+      - hexLiteral
+      - indexAlloc
+      - initClause
+      - methodExprCall
+      - nilValReturn
+      - octalLiteral
+      - offBy1
+      - rangeExprCopy
+      - regexpMust
+      - sloppyLen
+      - stringXbytes
+      - switchTrue
+      - typeAssertChain
+      - typeSwitchVar
+      - typeUnparen
+      - underef
+      - unlambda
+      - unnecessaryBlock
+      - unslice
+      - valSwap
+      - weakCond
+      - yodaStyleExpr
+
+      # Unused
+      # - appendAssign
+      # - commentFormatting
+      # - emptyStringTest
+      # - exitAfterDefer
+      # - ifElseChain
+      # - hugeParam
+      # - importShadow
+      # - nestingReduce
+      # - paramTypeCombine
+      # - ptrToRefParam
+      # - rangeValCopy
+      # - singleCaseSwitch
+      # - sloppyReassign
+      # - unlabelStmt
+      # - unnamedResult
+      # - wrapperFunc
+
+issues:
+  exclude:
+    - composites
+
+  exclude-rules:
+    # Exclude some linters from running on test files.
+    - path: _test\.go$|^tests/|^samples/
+      linters:
+        - goconst
+        - interfacer
+        - errcheck
+
+  # Independently from option `exclude` we use default exclude patterns,
+  # it can be disabled by this option. To list all
+  # excluded by default patterns execute `golangci-lint run --help`.
+  # Default value for this option is true.
+  exclude-use-default: true
+
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0

README.md

@@ -18,3 +18,11 @@
 * gripperPlay - test out gripper code
 * saveImageFromWebcam - really just to test out webcam capture code
 * vision - utilities for working with images to test out vision library code
+
+## Linting
+
+```
+go run github.com/golangci/golangci-lint/cmd/golangci-lint run -v ./...
+go get -u github.com/edaniels/golinters/cmd/combined
+go vet -vettool=$(which combined) ./...
+```

arm/ur.go

@@ -3,16 +3,20 @@ package arm
 import (
 	"encoding/binary"
 	"fmt"
+	"io"
 	"math"
 	"net"
 	"time"
+
+	"github.com/echolabsinc/robotcore/utils/log"
 )
 
 type URArm struct {
 	conn     net.Conn
 	State    RobotState
 	Debug    bool
 	haveData bool
+	logger   log.Logger
 }
 
 func (arm *URArm) Close() {
@@ -26,14 +30,14 @@ func URArmConnect(host string) (*URArm, error) {
 		return nil, err
 	}
 
-	arm := &URArm{conn: conn, Debug: false, haveData: false}
+	arm := &URArm{conn: conn, Debug: false, haveData: false, logger: log.Global}
 
 	go reader(conn, arm) // TODO: how to shutdown
 
 	slept := 0
 	for !arm.haveData {
 		time.Sleep(100 * time.Millisecond)
-		slept += 1
+		slept++
 
 		if slept > 20 {
 			return nil, fmt.Errorf("arm isn't respond")
@@ -78,7 +82,7 @@ func (arm *URArm) MoveToJointPositionRadians(radians []float64) error {
 		good := true
 		for idx, r := range radians {
 			if math.Round(r*100) != math.Round(arm.State.Joints[idx].Qactual*100) {
-				//fmt.Printf("joint %d want: %f have: %f\n", idx, r, arm.State.Joints[idx].Qactual)
+				//arm.logger.Debugf("joint %d want: %f have: %f\n", idx, r, arm.State.Joints[idx].Qactual)
 				good = false
 			}
 		}
@@ -134,7 +138,7 @@ func (arm *URArm) MoveToPosition(x, y, z, rx, ry, rz float64) error {
 		}
 
 		if slept > 10000 {
-			return fmt.Errorf("can't reach position.\n want: %f %f %f %f %f %f\n   at: %f %f %f %f %f %f\n",
+			return fmt.Errorf("can't reach position.\n want: %f %f %f %f %f %f\n   at: %f %f %f %f %f %f",
 				x, y, z, rx, ry, rz,
 				arm.State.CartesianInfo.X, arm.State.CartesianInfo.Y, arm.State.CartesianInfo.Z,
 				arm.State.CartesianInfo.Rx, arm.State.CartesianInfo.Ry, arm.State.CartesianInfo.Rz)
@@ -153,7 +157,7 @@ func (arm *URArm) AddToLog(msg string) error {
 	return err
 }
 
-func reader(conn net.Conn, arm *URArm) {
+func reader(conn io.Reader, arm *URArm) {
 	for {
 		buf := make([]byte, 4)
 		n, err := conn.Read(buf)
@@ -186,7 +190,7 @@ func reader(conn net.Conn, arm *URArm) {
 			arm.State = state
 			arm.haveData = true
 			if arm.Debug {
-				fmt.Printf("isOn: %v stopped: %v joints: %f %f %f %f %f %f cartesian: %f %f %f %f %f %f\n",
+				arm.logger.Debugf("isOn: %v stopped: %v joints: %f %f %f %f %f %f cartesian: %f %f %f %f %f %f\n",
 					state.RobotModeData.IsRobotPowerOn,
 					state.RobotModeData.IsEmergencyStopped || state.RobotModeData.IsProtectiveStopped,
 					state.Joints[0].AngleDegrees(),
@@ -210,7 +214,7 @@ func reader(conn net.Conn, arm *URArm) {
 		case 5: // MODBUS_INFO_MESSAGE
 			data := binary.BigEndian.Uint32(buf[1:])
 			if data != 0 {
-				fmt.Printf("got unexpected MODBUS_INFO_MESSAGE %d\n", data)
+				arm.logger.Debugf("got unexpected MODBUS_INFO_MESSAGE %d\n", data)
 			}
 
 		case 23: // SAFETY_SETUP_BROADCAST_MESSAGE
@@ -219,18 +223,18 @@ func reader(conn net.Conn, arm *URArm) {
 			break
 		case 25: // PROGRAM_STATE_MESSAGE
 			if len(buf) != 12 {
-				fmt.Println("got bad PROGRAM_STATE_MESSAGE ??")
+				arm.logger.Debug("got bad PROGRAM_STATE_MESSAGE ??")
 			} else {
 				a := binary.BigEndian.Uint32(buf[1:])
 				b := buf[9]
 				c := buf[10]
 				d := buf[11]
 				if a != 4294967295 || b != 1 || c != 0 || d != 0 {
-					fmt.Printf("got unknown PROGRAM_STATE_MESSAGE %v %v %v %v\n", a, b, c, d)
+					arm.logger.Debugf("got unknown PROGRAM_STATE_MESSAGE %v %v %v %v\n", a, b, c, d)
 				}
 			}
 		default:
-			fmt.Printf("ur: unknown messageType: %v size: %d %v\n", buf[0], len(buf), buf)
+			arm.logger.Debugf("ur: unknown messageType: %v size: %d %v\n", buf[0], len(buf), buf)
 		}
 	}
 }