From f344aeb946b92a16776d29976be1906dbc13a815 Mon Sep 17 00:00:00 2001 From: Joshua Rich Date: Sat, 8 Jun 2024 12:03:34 +1000 Subject: [PATCH] refactor(linux): :recycle: improve code readability --- internal/linux/device.go | 4 +++ internal/linux/device_test.go | 56 ++++++++++++++++------------- internal/linux/location/location.go | 27 ++++++++++---- internal/linux/sensor.go | 22 +++++++----- internal/linux/sensor_test.go | 35 +++++++++--------- internal/linux/worker.go | 41 +++++++++++++++++---- 6 files changed, 120 insertions(+), 65 deletions(-) diff --git a/internal/linux/device.go b/internal/linux/device.go index cef889b02..fee35adf5 100644 --- a/internal/linux/device.go +++ b/internal/linux/device.go @@ -166,6 +166,7 @@ func Chassis() string { // used for getting information on running apps. func FindPortal() string { desktop := os.Getenv("XDG_CURRENT_DESKTOP") + switch { case strings.Contains(desktop, "KDE"): return "org.freedesktop.impl.portal.desktop.kde" @@ -223,9 +224,12 @@ func GetDistroDetails() (name, version string) { } // GetKernelVersion will retrieve the kernel version. +// +//nolint:prealloc func GetKernelVersion() string { var utsname syscall.Utsname var versionBytes []byte + err := syscall.Uname(&utsname) if err != nil { log.Warn().Err(err).Msg("Could not retrieve kernel version.") diff --git a/internal/linux/device_test.go b/internal/linux/device_test.go index 1f5a072d2..52b21633c 100644 --- a/internal/linux/device_test.go +++ b/internal/linux/device_test.go @@ -3,6 +3,7 @@ // This software is released under the MIT License. // https://opensource.org/licenses/MIT +//nolint:paralleltest,dupl package linux import ( @@ -13,54 +14,58 @@ import ( "testing" mqtthass "github.com/joshuar/go-hass-anything/v9/pkg/hass" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/joshuar/go-hass-agent/internal/preferences" "github.com/joshuar/go-hass-agent/pkg/linux/whichdistro" ) -func compareDevice(t *testing.T, a, b *Device) bool { +func compareDevice(t *testing.T, got, want *Device) bool { + t.Helper() + switch { - case !reflect.DeepEqual(a.appName, b.appName): + case !reflect.DeepEqual(got.appName, want.appName): t.Error("appName does not match") return false - case !reflect.DeepEqual(a.appVersion, b.appVersion): + case !reflect.DeepEqual(got.appVersion, want.appVersion): t.Error("appVersion does not match") return false - case !reflect.DeepEqual(a.distro, b.distro): + case !reflect.DeepEqual(got.distro, want.distro): t.Error("distro does not match") return false - case !reflect.DeepEqual(a.distroVersion, b.distroVersion): + case !reflect.DeepEqual(got.distroVersion, want.distroVersion): t.Error("distroVersion does not match") return false - case !reflect.DeepEqual(a.hostname, b.hostname): + case !reflect.DeepEqual(got.hostname, want.hostname): t.Error("hostname does not match") return false - case !reflect.DeepEqual(a.hwModel, b.hwModel): + case !reflect.DeepEqual(got.hwModel, want.hwModel): t.Error("hwModel does not match") return false - case !reflect.DeepEqual(a.hwVendor, b.hwVendor): + case !reflect.DeepEqual(got.hwVendor, want.hwVendor): t.Error("hwVendor does not match") return false } return true } -func compareMQTTDevice(t *testing.T, a, b *mqtthass.Device) bool { +func compareMQTTDevice(t *testing.T, got, want *mqtthass.Device) bool { + t.Helper() + switch { - case !reflect.DeepEqual(a.Name, b.Name): + case !reflect.DeepEqual(got.Name, want.Name): t.Error("name does not match") return false - case !reflect.DeepEqual(a.URL, b.URL): + case !reflect.DeepEqual(got.URL, want.URL): t.Error("URL does not match") return false - case !reflect.DeepEqual(a.SWVersion, b.SWVersion): + case !reflect.DeepEqual(got.SWVersion, want.SWVersion): t.Error("SWVersion does not match") return false - case !reflect.DeepEqual(a.Manufacturer, b.Manufacturer): + case !reflect.DeepEqual(got.Manufacturer, want.Manufacturer): t.Error("Manufacturer does not match") return false - case !reflect.DeepEqual(a.Model, b.Model): + case !reflect.DeepEqual(got.Model, want.Model): t.Error("Model does not match") return false } @@ -92,9 +97,9 @@ func TestNewDevice(t *testing.T) { osReleaseFiles []string } tests := []struct { + want *Device name string args args - want *Device }{ { name: "with OS Release", @@ -139,8 +144,8 @@ func TestMQTTDevice(t *testing.T) { Identifiers: []string{dev.DeviceID()}, } tests := []struct { - name string want *mqtthass.Device + name string }{ { name: "default", @@ -156,6 +161,7 @@ func TestMQTTDevice(t *testing.T) { } } +//revive:disable:unhandled-error func TestFindPortal(t *testing.T) { type args struct { setup func() @@ -202,26 +208,26 @@ func TestGetDistroID(t *testing.T) { id := "testdistro" goodFile := filepath.Join(t.TempDir(), "goodfile") fh, err := os.Create(goodFile) - assert.Nil(t, err) + require.NoError(t, err) fmt.Fprintln(fh, `VERSION_ID="`+versionID+`"`) fmt.Fprintln(fh, `ID="`+id+`"`) fh.Close() tests := []struct { name string - wantId string + wantID string wantVersionid string osReleaseFile string }{ { name: "File exists", - wantId: id, + wantID: id, wantVersionid: versionID, osReleaseFile: goodFile, }, { name: "File does not exist.", - wantId: unknownDistro, + wantID: unknownDistro, wantVersionid: unknownDistroVersion, osReleaseFile: "/dev/null", }, @@ -229,9 +235,9 @@ func TestGetDistroID(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { whichdistro.OSReleaseFile = tt.osReleaseFile - gotId, gotVersionid := GetDistroID() - if gotId != tt.wantId { - t.Errorf("GetDistroID() gotId = %v, want %v", gotId, tt.wantId) + gotID, gotVersionid := GetDistroID() + if gotID != tt.wantID { + t.Errorf("GetDistroID() gotId = %v, want %v", gotID, tt.wantID) } if gotVersionid != tt.wantVersionid { t.Errorf("GetDistroID() gotVersionid = %v, want %v", gotVersionid, tt.wantVersionid) @@ -245,7 +251,7 @@ func TestGetDistroDetails(t *testing.T) { name := "Test Distro" goodFile := filepath.Join(t.TempDir(), "goodfile") fh, err := os.Create(goodFile) - assert.Nil(t, err) + require.NoError(t, err) fmt.Fprintln(fh, `VERSION="`+version+`"`) fmt.Fprintln(fh, `NAME="`+name+`"`) fh.Close() diff --git a/internal/linux/location/location.go b/internal/linux/location/location.go index b9c154d46..475518967 100644 --- a/internal/linux/location/location.go +++ b/internal/linux/location/location.go @@ -7,7 +7,6 @@ package location import ( "context" - "errors" "github.com/godbus/dbus/v5" "github.com/rs/zerolog/log" @@ -37,26 +36,30 @@ type locationSensor struct { linux.Sensor } -//revive:disable:unused-receiver +//nolint:unused-receiver func (s *locationSensor) Name() string { return "Location" } -//revive:disable:unused-receiver +//nolint:unused-receiver func (s *locationSensor) ID() string { return "location" } type worker struct { clientPath dbus.ObjectPath } +//nolint:exhaustruct func (w *worker) Setup(ctx context.Context) *dbusx.Watch { var err error w.clientPath, err = dbusx.GetData[dbus.ObjectPath](ctx, dbusx.SystemBus, managerPath, geoclueInterface, getClientCall) + if !w.clientPath.IsValid() || err != nil { log.Error().Err(err).Msg("Could not set up a geoclue client.") + return nil } if err = dbusx.SetProp(ctx, dbusx.SystemBus, string(w.clientPath), geoclueInterface, desktopIDProp, preferences.AppID); err != nil { log.Error().Err(err).Msg("Could not set a geoclue client id.") + return nil } @@ -73,6 +76,7 @@ func (w *worker) Setup(ctx context.Context) *dbusx.Watch { // Request to start tracking location updates. if err = dbusx.Call(ctx, dbusx.SystemBus, string(w.clientPath), geoclueInterface, startCall); err != nil { log.Warn().Err(err).Msg("Could not start geoclue client.") + return nil } @@ -91,14 +95,17 @@ func (w *worker) Watch(ctx context.Context, triggerCh chan dbusx.Trigger) chan s go func() { defer close(sensorCh) + for { select { case <-ctx.Done(): err := dbusx.Call(ctx, dbusx.SystemBus, string(w.clientPath), geoclueInterface, stopCall) if err != nil { log.Debug().Caller().Err(err).Msg("Failed to stop location updater.") + return } + return case event := <-triggerCh: if locationPath, ok := event.Content[1].(dbus.ObjectPath); ok { @@ -109,14 +116,16 @@ func (w *worker) Watch(ctx context.Context, triggerCh chan dbusx.Trigger) chan s } } }() + return sensorCh } //revive:disable:unused-receiver func (w *worker) Sensors(_ context.Context) ([]sensor.Details, error) { - return nil, errors.New("unimplemented") + return nil, linux.ErrUnimplemented } +//nolint:exhaustruct func NewLocationWorker() (*linux.SensorWorker, error) { return &linux.SensorWorker{ WorkerName: "Location Sensor", @@ -126,22 +135,26 @@ func NewLocationWorker() (*linux.SensorWorker, error) { nil } +//nolint:exhaustruct func newLocation(ctx context.Context, locationPath dbus.ObjectPath) *locationSensor { getProp := func(prop string) float64 { value, err := dbusx.GetProp[float64](ctx, dbusx.SystemBus, string(locationPath), geoclueInterface, locationInterface+"."+prop) if err != nil { log.Debug().Caller().Err(err). Msgf("Could not retrieve %s.", prop) + return 0 } + return value } - s := &locationSensor{} - s.Value = &sensor.LocationRequest{ + location := &locationSensor{} + location.Value = &sensor.LocationRequest{ Gps: []float64{getProp("Latitude"), getProp("Longitude")}, GpsAccuracy: int(getProp("Accuracy")), Speed: int(getProp("Speed")), Altitude: int(getProp("Altitude")), } - return s + + return location } diff --git a/internal/linux/sensor.go b/internal/linux/sensor.go index cfdec4542..eb4748afb 100644 --- a/internal/linux/sensor.go +++ b/internal/linux/sensor.go @@ -6,6 +6,7 @@ package linux import ( + "errors" "fmt" "strings" @@ -20,6 +21,8 @@ const ( DataSrcSysfs = "SysFS" ) +var ErrUnimplemented = errors.New("unimplemented functionality") + // Sensor represents a generic sensor on the Linux platform. Most sensors // will be able to use this struct, which satisfies the sensor.Sensor // interface, alllowing them to be sent as a sensor to Home Assistant. @@ -84,7 +87,7 @@ func (l *Sensor) Units() string { func (l *Sensor) Attributes() any { if l.SensorSrc != "" { return struct { - DataSource string `json:"Data Source"` + DataSource string `json:"data_source"` }{ DataSource: l.SensorSrc, } @@ -93,20 +96,21 @@ func (l *Sensor) Attributes() any { } func (l *Sensor) String() string { - var s strings.Builder - fmt.Fprintf(&s, "Name: %s (ID: %s)", l.Name(), l.ID()) + var sensorStr strings.Builder + + fmt.Fprintf(&sensorStr, "Name: %s (ID: %s)", l.Name(), l.ID()) if l.DeviceClass() > 0 { - fmt.Fprintf(&s, " Device Class: %s", l.DeviceClass().String()) + fmt.Fprintf(&sensorStr, " Device Class: %s", l.DeviceClass().String()) } if l.StateClass() > 0 { - fmt.Fprintf(&s, " State Class: %s", l.DeviceClass().String()) + fmt.Fprintf(&sensorStr, " State Class: %s", l.DeviceClass().String()) } - fmt.Fprintf(&s, " Value: %v", l.Value) + fmt.Fprintf(&sensorStr, " Value: %v", l.Value) if l.UnitsString != "" { - fmt.Fprintf(&s, " %s", l.UnitsString) + fmt.Fprintf(&sensorStr, " %s", l.UnitsString) } if l.Attributes() != nil { - fmt.Fprintf(&s, " Attributes: %v", l.Attributes()) + fmt.Fprintf(&sensorStr, " Attributes: %v", l.Attributes()) } - return s.String() + return sensorStr.String() } diff --git a/internal/linux/sensor_test.go b/internal/linux/sensor_test.go index ec2e7d69d..fc11a1f38 100644 --- a/internal/linux/sensor_test.go +++ b/internal/linux/sensor_test.go @@ -3,6 +3,7 @@ // This software is released under the MIT License. // https://opensource.org/licenses/MIT +//nolint:dupl,paralleltest,exhaustruct package linux import ( @@ -26,8 +27,8 @@ func TestSensor_Name(t *testing.T) { } tests := []struct { name string - fields fields want string + fields fields }{ { name: "known sensor type", @@ -41,7 +42,7 @@ func TestSensor_Name(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l := &Sensor{ + sensor := &Sensor{ Value: tt.fields.Value, IconString: tt.fields.IconString, UnitsString: tt.fields.UnitsString, @@ -52,7 +53,7 @@ func TestSensor_Name(t *testing.T) { DeviceClassValue: tt.fields.DeviceClassValue, StateClassValue: tt.fields.StateClassValue, } - if got := l.Name(); got != tt.want { + if got := sensor.Name(); got != tt.want { t.Errorf("Sensor.Name() = %v, want %v", got, tt.want) } }) @@ -73,8 +74,8 @@ func TestSensor_ID(t *testing.T) { } tests := []struct { name string - fields fields want string + fields fields }{ { name: "known sensor type", @@ -88,7 +89,7 @@ func TestSensor_ID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l := &Sensor{ + sensor := &Sensor{ Value: tt.fields.Value, IconString: tt.fields.IconString, UnitsString: tt.fields.UnitsString, @@ -99,7 +100,7 @@ func TestSensor_ID(t *testing.T) { DeviceClassValue: tt.fields.DeviceClassValue, StateClassValue: tt.fields.StateClassValue, } - if got := l.ID(); got != tt.want { + if got := sensor.ID(); got != tt.want { t.Errorf("Sensor.ID() = %v, want %v", got, tt.want) } }) @@ -119,9 +120,9 @@ func TestSensor_State(t *testing.T) { StateClassValue types.StateClass } tests := []struct { + want any name string fields fields - want any }{ { name: "known value", @@ -135,7 +136,7 @@ func TestSensor_State(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l := &Sensor{ + sensor := &Sensor{ Value: tt.fields.Value, IconString: tt.fields.IconString, UnitsString: tt.fields.UnitsString, @@ -146,7 +147,7 @@ func TestSensor_State(t *testing.T) { DeviceClassValue: tt.fields.DeviceClassValue, StateClassValue: tt.fields.StateClassValue, } - if got := l.State(); !reflect.DeepEqual(got, tt.want) { + if got := sensor.State(); !reflect.DeepEqual(got, tt.want) { t.Errorf("Sensor.State() = %v, want %v", got, tt.want) } }) @@ -182,7 +183,7 @@ func TestSensor_SensorType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l := &Sensor{ + sensor := &Sensor{ Value: tt.fields.Value, IconString: tt.fields.IconString, UnitsString: tt.fields.UnitsString, @@ -193,7 +194,7 @@ func TestSensor_SensorType(t *testing.T) { DeviceClassValue: tt.fields.DeviceClassValue, StateClassValue: tt.fields.StateClassValue, } - if got := l.SensorType(); !reflect.DeepEqual(got, tt.want) { + if got := sensor.SensorType(); !reflect.DeepEqual(got, tt.want) { t.Errorf("Sensor.SensorType() = %v, want %v", got, tt.want) } }) @@ -214,8 +215,8 @@ func TestSensor_Category(t *testing.T) { } tests := []struct { name string - fields fields want string + fields fields }{ { name: "default category", @@ -229,7 +230,7 @@ func TestSensor_Category(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l := &Sensor{ + sensor := &Sensor{ Value: tt.fields.Value, IconString: tt.fields.IconString, UnitsString: tt.fields.UnitsString, @@ -240,7 +241,7 @@ func TestSensor_Category(t *testing.T) { DeviceClassValue: tt.fields.DeviceClassValue, StateClassValue: tt.fields.StateClassValue, } - if got := l.Category(); got != tt.want { + if got := sensor.Category(); got != tt.want { t.Errorf("Sensor.Category() = %v, want %v", got, tt.want) } }) @@ -260,9 +261,9 @@ func TestSensor_Attributes(t *testing.T) { StateClassValue types.StateClass } tests := []struct { + want any name string fields fields - want any }{ { name: "with source", @@ -276,7 +277,7 @@ func TestSensor_Attributes(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - l := &Sensor{ + sensor := &Sensor{ Value: tt.fields.Value, IconString: tt.fields.IconString, UnitsString: tt.fields.UnitsString, @@ -287,7 +288,7 @@ func TestSensor_Attributes(t *testing.T) { DeviceClassValue: tt.fields.DeviceClassValue, StateClassValue: tt.fields.StateClassValue, } - if got := l.Attributes(); !reflect.DeepEqual(got, tt.want) { + if got := sensor.Attributes(); !reflect.DeepEqual(got, tt.want) { t.Errorf("Sensor.Attributes() = %v, want %v", got, tt.want) } }) diff --git a/internal/linux/worker.go b/internal/linux/worker.go index f83251a43..7fdb8e438 100644 --- a/internal/linux/worker.go +++ b/internal/linux/worker.go @@ -18,6 +18,8 @@ import ( "github.com/joshuar/go-hass-agent/pkg/linux/dbusx" ) +var ErrUnknownWorker = errors.New("unknown sensor worker type") + // pollingType interface represents sensors that are generated on some poll interval. type pollingType interface { Interval() time.Duration @@ -51,7 +53,7 @@ type SensorWorker struct { // Value is a pointer to an interface that exposes methods to retrieve the // sensor values for this worker. Value any - // WorkerName is a a short name to refer to this group of sensors. + // WorkerName is a short name to refer to this group of sensors. WorkerName string // WorkerDesc describes what the sensors measure. WorkerDesc string @@ -73,22 +75,45 @@ func (w *SensorWorker) Description() string { func (w *SensorWorker) Sensors(ctx context.Context) ([]sensor.Details, error) { switch worker := w.Value.(type) { case pollingType: - return worker.Sensors(ctx, 0) + sensors, err := worker.Sensors(ctx, 0) + if err != nil { + return nil, fmt.Errorf("failed to get current state of polling sensors: %w", err) + } + + return sensors, nil case dbusType: - return worker.Sensors(ctx) + sensors, err := worker.Sensors(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get current state of polling sensors: %w", err) + } + + return sensors, nil case eventType: - return worker.Sensors(ctx) + sensors, err := worker.Sensors(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get current state of polling sensors: %w", err) + } + + return sensors, nil case oneShotType: - return worker.Sensors(ctx) + sensors, err := worker.Sensors(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get current state of polling sensors: %w", err) + } + + return sensors, nil } - return nil, errors.New("unknown worker type") + return nil, ErrUnknownWorker } // Updates returns a channel on which sensor updates can be received. If the // functionality to send sensor updates cannot be achieved, it will return a // non-nil error. +// +//nolint:cyclop func (w *SensorWorker) Updates(ctx context.Context) (<-chan sensor.Details, error) { outCh := make(chan sensor.Details) + switch worker := w.Value.(type) { case pollingType: updater := func(d time.Duration) { @@ -113,6 +138,7 @@ func (w *SensorWorker) Updates(ctx context.Context) (<-chan sensor.Details, erro } go func() { defer close(outCh) + for s := range worker.Watch(ctx, eventCh) { outCh <- s } @@ -120,6 +146,7 @@ func (w *SensorWorker) Updates(ctx context.Context) (<-chan sensor.Details, erro case eventType: go func() { defer close(outCh) + for s := range worker.Events(ctx) { outCh <- s } @@ -137,7 +164,7 @@ func (w *SensorWorker) Updates(ctx context.Context) (<-chan sensor.Details, erro } }() default: - return nil, fmt.Errorf("unknown or unsupported worker type: %T", w.Value) + return nil, ErrUnknownWorker } return outCh, nil }