Skip to content

Rewrite in Golang#2

Open
martialblog wants to merge 2 commits into
mainfrom
golang
Open

Rewrite in Golang#2
martialblog wants to merge 2 commits into
mainfrom
golang

Conversation

@martialblog

Copy link
Copy Markdown
Member

No description provided.

martialblog and others added 2 commits June 29, 2026 12:56
Co-authored-by: Christoph Breit <christoph.breit@netways.de>
@martialblog martialblog self-assigned this Jun 29, 2026
Comment thread cmd/root.go
}

// checkContent compares the file content against the given thresholds
func checkContent(fileContent string, warningThreshold check.Threshold, criticalThreshold check.Threshold) *result.PartialResult {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should return an unknown if the file content is not a statfile

echo "foobar" > /tmp/status 

go run main.go  --statusfile /tmp/status
[CRITICAL] - Registered on network '' with signal strength 0%
\_ [OK] Status file was last updated 6 seconds ago
\_ [CRITICAL] Registered on network '' with signal strength 0%
|dbm=-113 signal=0%;40:;20: bit_error_rate=0

Comment thread cmd/root.go
"github.com/spf13/cobra"
)

var warning, critical, statusfilePath string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put those variables in a config struct so it obvious that they belong together and share a purpose.

Comment thread cmd/root.go
var csqRe = regexp.MustCompile(`\+CSQ:\s*(\d+),(\d+)`)
var copsReQuoted = regexp.MustCompile(`\+COPS:\s*\d+,\d+,"([^"]*)"`)
var copsReUnquoted = regexp.MustCompile(`\+COPS:\s*\d+,\d+,([^"]+)`)
var cregRe = regexp.MustCompile(`\+CREG:\s*(\d,(?:1|5))`)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could those be constants?

Comment thread cmd/root.go

var rootCmd = &cobra.Command{
Use: "check_sms3status",
Short: "This plugin checks the status of an SMS modem using the regular_run functionality provided in smstools3",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some prefix for "plugin" would be good. What kind of "plugin" is it? For what?

Assume someone might find this without a big Icinga/Nagios/Naemon background.

Comment thread cmd/root.go

var fileInfo os.FileInfo

if contentErr != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error check should be directly below the statement that might cause the error.

Comment thread cmd/root.go
val, err := strconv.Atoi(matches[1])
if err == nil {
signal = val
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no match for the regex, signal is just zero. Is that correct?

Comment thread cmd/root.go

msg := fmt.Sprintf("Registered on network '%s' with signal strength %0.f%%", network, sigproc)

result.Output = msg

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just result.Output = fmt.Sprinf....?

@RincewindsHat

Copy link
Copy Markdown
Member

Why move the files to the legacy directory? This is a git repo, they can just be removed here.

@martialblog

Copy link
Copy Markdown
Member Author

Why move the files to the legacy directory? This is a git repo, they can just be removed here.

True. We've done this before, I think so that the old code is more visible. We can still remove it at some point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants