Add netgear plugin #6

DsgnrFH · 2025-11-06T11:22:13Z

Squashed commits from main and from before

yhabteab · 2025-11-06T11:39:10Z

Sorry @oxzi, it's my fault :)! After force purshing to the main branch GitHub automatically closed #4 and can't also be reopened.

oxzi · 2025-11-06T13:37:35Z

Sorry @oxzi, it's my fault :)! After force purshing to the main branch GitHub automatically closed #4 and can't also be reopened.

Happens, I guess (:

@DsgnrFH: By having a new PR, all my old comments are now kinda void. Could you nevertheless comment under each of my comments in the other PR #4 if you have addressed it? I am going to take a closer look at this PR within this week, most likely tomorrow.

DsgnrFH · 2025-11-07T08:08:06Z

Sorry @oxzi, it's my fault :)! After force purshing to the main branch GitHub automatically closed #4 and can't also be reopened.

Happens, I guess (:

@DsgnrFH: By having a new PR, all my old comments are now kinda void. Could you nevertheless comment under each of my comments in the other PR #4 if you have addressed it? I am going to take a closer look at this PR within this week, most likely tomorrow.

I believe I have addressed everything you suggested

internal/checks/checks.go

netgear/netgear.go

main.go

netgear/types.go

internal/checks/checks.go

netgear/netgear.go

main.go

yhabteab

@oxzi these are my last review comments, so you can also have look at this, I won't intervene further :-)

yhabteab · 2025-11-07T16:25:29Z

internal/checks/checks.go

+		addPerfSubcheck("IN", inLoss, inStatus)
+		addPerfSubcheck("OUT", outLoss, outStatus)


Suggested change

addPerfSubcheck("IN", inLoss, inStatus)

addPerfSubcheck("OUT", outLoss, outStatus)

if err := addPerfSubcheck("IN", inLoss, inStatus); err != nil {

return nil, err

}

if err := addPerfSubcheck("OUT", outLoss, outStatus); err != nil {

return nil, err

}

yhabteab · 2025-11-07T16:28:06Z

main.go

+	hidetemp := flag.Bool("notemp", false, "Hide the Temperature info")
+	hidefans := flag.Bool("nofans", false, "Hide the Fans info")
+
+	var mode stringSliceFlag = []string{"basic"}


Suggested change

var mode stringSliceFlag = []string{"basic"}

mode := stringSliceFlag{"basic"}

yhabteab · 2025-11-07T16:28:51Z

main.go

+	statsWarn := flag.Float64("stats-warning", 5, "Port stats warning threshold")
+	statsCrit := flag.Float64("stats-critical", 20, "Port stats critical threshold")
+
+	var portsToCheck intSliceFlag = []int{1, 2, 3, 4, 5, 6, 7, 8}


Suggested change

var portsToCheck intSliceFlag = []int{1, 2, 3, 4, 5, 6, 7, 8}

portsToCheck := intSliceFlag{1, 2, 3, 4, 5, 6, 7, 8}

yhabteab · 2025-11-07T16:30:24Z

main.go

+		cpuUsage, _ := netgear.StringPercentToFloat(deviceInfo.DeviceInfo.Cpu[0].Usage)
+		memUsage, _ := netgear.StringPercentToFloat(deviceInfo.DeviceInfo.Memory[0].Usage)


You're still ignoring errors.

main.go

oxzi · 2025-11-10T10:57:44Z

main.go

+	if *help || *username == "" || *password == "" {
+		flag.Usage()
+		return
+	}


I would split these up in two cases. If someone request the help, it's okay. However, if someone forgets to set credentials - or they somehow get lost along the way -, it is an error.

Suggested change

if *help || *username == "" || *password == "" {

flag.Usage()

return

}

if *help {

flag.Usage()

return

}

if *username == "" || *password == "" {

fmt.Println("Both username and password are required")

os.Exit(check.Unknown)

}

oxzi · 2025-11-10T11:00:26Z

main.go

+		upTime := deviceInfo.DeviceInfo.Details[0].Uptime
+		cpuUsage, _ := netgear.StringPercentToFloat(deviceInfo.DeviceInfo.Cpu[0].Usage)
+		memUsage, _ := netgear.StringPercentToFloat(deviceInfo.DeviceInfo.Memory[0].Usage)
+		fan := deviceInfo.DeviceInfo.Fan[0].Details[0]
+		fanName := fan.Description
+		fanSpeed := fan.Speed
+		sensorDetails := deviceInfo.DeviceInfo.Sensor[0].Details


Are you sure that there is always an element at index 0? What if the API slightly changes or something along the way has a bad day.

Please consider checking if the slice is long enough before accessing something. Otherwise, if you say that this MUST work, consider adding a deferred recover() call, which prints the error and exits with UNKNOWN.

oxzi · 2025-11-10T11:01:12Z

main.go

+			tempPartial, err := checks.CheckTemperature(sensorDetails, *tempWarn, *tempCrit)
+			if err != nil {
+				errRes := result.NewPartialResult()
+				errRes.Output = fmt.Sprintf("Temperatuer check error: %v", err)


Suggested change

errRes.Output = fmt.Sprintf("Temperatuer check error: %v", err)

errRes.Output = fmt.Sprintf("Temperature check error: %v", err)

oxzi · 2025-11-10T11:10:49Z

netgear/netgear.go

+	const defaultPageIndex = 1
+	const defaultPageSize = 25
+
+	u := n.baseUrl.JoinPath("port_statistics")
+	q := u.Query()
+	q.Set("type", statType)
+	q.Set("indexPage", strconv.Itoa(defaultPageIndex))
+	q.Set("pageSize", strconv.Itoa(defaultPageSize))


As commented in the old PR, this pagination looks a bit weird.

What about ports on page 2? Can there be a second page? Does this mean we are only getting information about 25 ports? If so, lots of switches would miss information.

oxzi · 2025-11-10T11:11:23Z

netgear/netgear.go

+func NewNetgear(hostName, username, password string) (*Netgear, error) {
+	u, err := url.Parse(strings.TrimSpace(hostName))


#4 (comment)

Suggested change

func NewNetgear(hostName, username, password string) (*Netgear, error) {

u, err := url.Parse(strings.TrimSpace(hostName))

func NewNetgear(baseUrl, username, password string) (*Netgear, error) {

u, err := url.Parse(strings.TrimSpace(baseUrl))

Add netgear plugin

dbbce72

DsgnrFH requested review from oxzi and yhabteab November 6, 2025 11:22

DsgnrFH self-assigned this Nov 6, 2025

cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Nov 6, 2025

yhabteab reviewed Nov 7, 2025

View reviewed changes

main.go Outdated Show resolved Hide resolved

netgear/types.go Show resolved Hide resolved

yhabteab mentioned this pull request Nov 7, 2025

Restructured the whole code #4

Closed

Addressed some more comments and advises

5a781b2

DsgnrFH requested a review from yhabteab November 7, 2025 14:42

yhabteab reviewed Nov 7, 2025

View reviewed changes

internal/checks/checks.go Outdated Show resolved Hide resolved

internal/checks/checks.go Outdated Show resolved Hide resolved

internal/checks/checks.go Outdated Show resolved Hide resolved

netgear/netgear.go Outdated Show resolved Hide resolved

main.go Outdated Show resolved Hide resolved

yhabteab reviewed Nov 7, 2025

View reviewed changes

main.go Outdated Show resolved Hide resolved

DsgnrFH added 4 commits November 7, 2025 16:45

Addressed even more suggestions

74102ea

Fixed main

e4b4732

Removed unused import

f8bbdcb

Addressed more comments

cf28c82

yhabteab reviewed Nov 7, 2025

View reviewed changes

oxzi reviewed Nov 10, 2025

View reviewed changes

		addPerfSubcheck("IN", inLoss, inStatus)
		addPerfSubcheck("OUT", outLoss, outStatus)

-		addPerfSubcheck("IN", inLoss, inStatus)
-		addPerfSubcheck("OUT", outLoss, outStatus)
+		if err := addPerfSubcheck("IN", inLoss, inStatus); err != nil {
+			return nil, err
+		}
+		if err := addPerfSubcheck("OUT", outLoss, outStatus); err != nil {
+			return nil, err
+		}

	var mode stringSliceFlag = []string{"basic"}
	mode := stringSliceFlag{"basic"}

	var portsToCheck intSliceFlag = []int{1, 2, 3, 4, 5, 6, 7, 8}
	portsToCheck := intSliceFlag{1, 2, 3, 4, 5, 6, 7, 8}

		cpuUsage, _ := netgear.StringPercentToFloat(deviceInfo.DeviceInfo.Cpu[0].Usage)
		memUsage, _ := netgear.StringPercentToFloat(deviceInfo.DeviceInfo.Memory[0].Usage)

-	if *help || *username == "" || *password == "" {
-		flag.Usage()
-		return
-	}
+	if *help {
+		flag.Usage()
+		return
+	}
+	if *username == "" || *password == "" {
+		fmt.Println("Both username and password are required")
+		os.Exit(check.Unknown)
+	}

	errRes.Output = fmt.Sprintf("Temperatuer check error: %v", err)
	errRes.Output = fmt.Sprintf("Temperature check error: %v", err)

		func NewNetgear(hostName, username, password string) (*Netgear, error) {
		u, err := url.Parse(strings.TrimSpace(hostName))

Add netgear plugin #6

Are you sure you want to change the base?

Add netgear plugin #6

Conversation

DsgnrFH commented Nov 6, 2025

Uh oh!

yhabteab commented Nov 6, 2025

Uh oh!

oxzi commented Nov 6, 2025

Uh oh!

DsgnrFH commented Nov 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yhabteab left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants