Improve build pipelines and code quality

[sansmoraxz]

Fixes: #131

Added golangci-lint with a couple of rules that made sense.

And their related fixes.

• revive: the defacto golint replacement
• govet: just simplified go vet
• gosec: Scan for potential security issues. Did catch a couple of casting related bugs in code.
• gocritic: highly configurable. did catch the exit and defer skip in cli for err case and some comment spacing.
• staticcheck: https://staticcheck.dev/
• errcheck: catch where you forgot to handle err returned
• dupl: Code deduplication scanner.

You can check other rules here: https://golangci-lint.run/docs/linters/configuration.

Updated makefile to use golangci-lint instead of golint.

Also CI pipeline has really old versions of golangci-lint.

docker run --rm public.ecr.aws/gridx/base-images:modbus-dev-1.21.latest golangci-lint --version
golangci-lint has version 1.56.1 built with go1.22.0 from a25592b5 on 2024-02-08T18:03:33Z

The provided .golangci.yml would not be accepted by that version. Ensure to update before merging.

Maintainers already granted write access to this branch.

[leonklingele]

Thanks for the PR, @sansmoraxz! 😊 Always very happy to see your contributions to the repo!

Left some smaller nits but also changes which, imo, are blockers. Definitely open to discuss things though! 😃

[leonklingele]

To other reviewers wondering why this change was made:

Why: To make the developer's intent explicitly clear and visually symmetrical.

In Modbus RTU, the 16-bit CRC (Cyclic Redundancy Check) is appended to the end of the packet, sending the lower byte first, followed by the upper byte.

In Go, converting a 16-bit integer to a byte using byte(checksum) automatically drops the higher 8 bits and keeps the lower 8 bits. It works perfectly fine on its own.

However, by adding the bitwise AND mask (& 0x00ff), the code explicitly states: "I am intentionally extracting the lower 8 bits here."

It also provides nice visual symmetry with the line immediately above it (byte(checksum >> 8)), showing exactly how the 16 bits are being split into two 8-bit chunks. Additionally, explicit masking like this often pacifies strict static analysis tools (linters) that might warn about implicit data truncation.

Thanks @sansmoraxz! 😊

[leonklingele]

TODO @leonklingele: Review this.

[leonklingele]

TODO @leonklingele: Lots of changes, still open for review.

[leonklingele]

All read funcs (and some of the write funcs) still use common / duplicate code. For consistency, I'd prefer to either duplicate code everywhere (as before), or unify it for all read + write funcs.

[sansmoraxz]

I was thinking, perhaps it would be better to have typed errors instead of string builders scattered across codebase. WDYT?

Anyway might be better to keep it out of this PR, as that's a large behaviour change.