jaypipes / ghw

Go HardWare discovery/inspection library
Apache License 2.0
1.62k stars 174 forks source link

Add AutoNegotiation and PauseFrameUse NICCapabilities #338

Closed jak3kaj closed 1 year ago

jak3kaj commented 1 year ago

Make AutoNegotiation and PauseFrameUse NICCapability objects:

Refactoring PR #335

jak3kaj commented 1 year ago

By moving setNicAttrEthtool() into netDeviceCapabilities() I should have resolved the concerns you shared.

I made some more changes as the logic made less sense after moving the code. I split up the updateNicAttrEthtool() method and created autoNegCap() and pauseFrameUseCap() so they behaved more like the other code in netDeviceCapabilities() (returning *NICCapabilties objects).

I put the logic to parse ethtool before parsing ethtool -k so there is a sort of logical order (ASCII-betical).

If I were to replace running ethtool with an ethtool-like go library, hopefully this function (and functions it calls) are all that would need to be modified. In the back of my mind I imagine that calling a go library can return the same map that parseNicAttrEthtool() is returning, so we can use autoNegCap() and pauseFrameUseCap() in the same way.

jaypipes commented 1 year ago

@jak3kaj can you run gofmt -w . on your source tree checkout please? the formatting check is complaining...

jak3kaj commented 1 year ago

The thing it's flagging is not flagged by gofmt or go fmt.

It wants this:

            expected: []*NICCapability{
                {
                    Name:      "auto-negotiation",
                    IsEnabled: true,
                    CanEnable: true,
                },
                {
                    Name:      "pause-frame-use",
                    IsEnabled: false,
                    CanEnable: false,
                },
            },

instead of this:

            expected: []*NICCapability{
                &NICCapability{
                    Name:      "auto-negotiation",
                    IsEnabled: true,
                    CanEnable: true,
                },
                &NICCapability{
                    Name:      "pause-frame-use",
                    IsEnabled: false,
                    CanEnable: false,
                },
            },

🤷 I will bow to the formatter script

jak3kaj commented 1 year ago

oops, I just fixed the commit that wasn't signed off and rebased with main. This should be ready to merge now.