Removing some code smell reported by SonarCloud (#132)

* changing lots of if else to switch in status * removing logging of user-provided * removing tons of ifs for a map and a loop
2026-02-27 09:54:18 +08:00 · 2022-01-11 21:24:31 +01:00
parent 04ac282d86
commit b6ce2b89b9
7 changed files with 148 additions and 156 deletions
--- a/src/CommandSupport.go
+++ b/src/CommandSupport.go
@@ -13,137 +13,140 @@ import (
 // initCommandAllowList func
 func initCommandAllowList() {

-	// allow all commands available below
-	allowAll := getEnvAsBool("COMMANDS_ALL", false)
+	// generate map of all available commands
+	CommandList := make(map[string][]string)

 	// https://github.com/adriankumpf/teslamate/discussions/1433
-	if getEnvAsBool("COMMANDS_LOGGING", false) || allowAll {
-		allowList = append(allowList,
-			"/logging/resume",
-			"/logging/suspend")
+	CommandList["COMMANDS_LOGGING"] = []string{
+		"/logging/resume",
+		"/logging/suspend",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/wake
-	if getEnvAsBool("COMMANDS_WAKE", false) || allowAll {
-		allowList = append(allowList, "/wake_up")
+	CommandList["COMMANDS_WAKE"] = []string{
+		"/wake_up",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/alerts
-	if getEnvAsBool("COMMANDS_ALERT", false) || allowAll {
-		allowList = append(allowList,
-			"/command/honk_horn",
-			"/command/flash_lights")
+	CommandList["COMMANDS_ALERT"] = []string{
+		"/command/honk_horn",
+		"/command/flash_lights",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/remotestart
-	if getEnvAsBool("COMMANDS_REMOTESTART", false) || allowAll {
-		allowList = append(allowList, "/command/remote_start_drive")
+	CommandList["COMMANDS_REMOTESTART"] = []string{
+		"/command/remote_start_drive",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/homelink
-	if getEnvAsBool("COMMANDS_HOMELINK", false) || allowAll {
-		allowList = append(allowList, "/command/trigger_homelink")
+	CommandList["COMMANDS_HOMELINK"] = []string{
+		"/command/trigger_homelink",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/speedlimit
-	if getEnvAsBool("COMMANDS_SPEEDLIMIT", false) || allowAll {
-		allowList = append(allowList,
-			"/command/speed_limit_set_limit",
-			"/command/speed_limit_activate",
-			"/command/speed_limit_deactivate",
-			"/command/speed_limit_clear_pin")
+	CommandList["COMMANDS_SPEEDLIMIT"] = []string{
+		"/command/speed_limit_set_limit",
+		"/command/speed_limit_activate",
+		"/command/speed_limit_deactivate",
+		"/command/speed_limit_clear_pin",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/valet
-	if getEnvAsBool("COMMANDS_VALET", false) || allowAll {
-		allowList = append(allowList,
-			"/command/set_valet_mode",
-			"/command/reset_valet_pin")
+	CommandList["COMMANDS_VALET"] = []string{
+		"/command/set_valet_mode",
+		"/command/reset_valet_pin",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/sentrymode
-	if getEnvAsBool("COMMANDS_SENTRYMODE", false) || allowAll {
-		allowList = append(allowList, "/command/set_sentry_mode")
+	CommandList["COMMANDS_SENTRYMODE"] = []string{
+		"/command/set_sentry_mode",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/doors
-	if getEnvAsBool("COMMANDS_DOORS", false) || allowAll {
-		allowList = append(allowList,
-			"/command/door_unlock",
-			"/command/door_lock")
+	CommandList["COMMANDS_DOORS"] = []string{
+		"/command/door_unlock",
+		"/command/door_lock",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/trunk
-	if getEnvAsBool("COMMANDS_TRUNK", false) || allowAll {
-		allowList = append(allowList, "/command/actuate_trunk")
+	CommandList["COMMANDS_TRUNK"] = []string{
+		"/command/actuate_trunk",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/windows
-	if getEnvAsBool("COMMANDS_WINDOWS", false) || allowAll {
-		allowList = append(allowList, "/command/window_control")
+	CommandList["COMMANDS_WINDOWS"] = []string{
+		"/command/window_control",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/sunroof
-	if getEnvAsBool("COMMANDS_SUNROOF", false) || allowAll {
-		allowList = append(allowList, "/command/sun_roof_control")
+	CommandList["COMMANDS_SUNROOF"] = []string{
+		"/command/sun_roof_control",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/charging
-	if getEnvAsBool("COMMANDS_CHARGING", false) || allowAll {
-		allowList = append(allowList,
-			"/command/charge_port_door_open",
-			"/command/charge_port_door_close",
-			"/command/charge_start",
-			"/command/charge_stop",
-			"/command/set_charge_limit",
-			"/command/set_charging_amps",
-			"/command/set_scheduled_charging",
-			"/command/set_scheduled_departure")
+	CommandList["COMMANDS_CHARGING"] = []string{
+		"/command/charge_port_door_open",
+		"/command/charge_port_door_close",
+		"/command/charge_start",
+		"/command/charge_stop",
+		"/command/set_charge_limit",
+		"/command/set_charging_amps",
+		"/command/set_scheduled_charging",
+		"/command/set_scheduled_departure",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/climate
-	if getEnvAsBool("COMMANDS_CLIMATE", false) || allowAll {
-		allowList = append(allowList,
-			"/command/auto_conditioning_start",
-			"/command/auto_conditioning_stop",
-			"/command/set_temps",
-			"/command/set_preconditioning_max",
-			"/command/remote_seat_heater_request",
-			"/command/remote_steering_wheel_heater_request",
-			"/command/set_bioweapon_mode")
+	CommandList["COMMANDS_CLIMATE"] = []string{
+		"/command/auto_conditioning_start",
+		"/command/auto_conditioning_stop",
+		"/command/set_temps",
+		"/command/set_preconditioning_max",
+		"/command/remote_seat_heater_request",
+		"/command/remote_steering_wheel_heater_request",
+		"/command/set_bioweapon_mode",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/media
-	if getEnvAsBool("COMMANDS_MEDIA", false) || allowAll {
-		allowList = append(allowList,
-			"/command/media_toggle_playback",
-			"/command/media_next_track",
-			"/command/media_prev_track",
-			"/command/media_next_fav",
-			"/command/media_prev_fav",
-			"/command/media_volume_up",
-			"/command/media_volume_down")
+	CommandList["COMMANDS_MEDIA"] = []string{
+		"/command/media_toggle_playback",
+		"/command/media_next_track",
+		"/command/media_prev_track",
+		"/command/media_next_fav",
+		"/command/media_prev_fav",
+		"/command/media_volume_up",
+		"/command/media_volume_down",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/sharing
-	if getEnvAsBool("COMMANDS_SHARING", false) || allowAll {
-		allowList = append(allowList,
-			"/command/share",
-			"/command/navigation_sc_request")
+	CommandList["COMMANDS_SHARING"] = []string{
+		"/command/share",
+		"/command/navigation_sc_request",
 	}

 	// https://tesla-api.timdorr.com/vehicle/commands/softwareupdate
-	if getEnvAsBool("COMMANDS_SOFTWAREUPDATE", false) || allowAll {
-		allowList = append(allowList,
-			"/command/schedule_software_update",
-			"/command/cancel_software_update")
+	CommandList["COMMANDS_SOFTWAREUPDATE"] = []string{
+		"/command/schedule_software_update",
+		"/command/cancel_software_update",
 	}

 	// not documentet and unsorted new endpoints
-	if getEnvAsBool("COMMANDS_UNKNOWN", false) || allowAll {
-		allowList = append(allowList,
-			"/command/upcoming_calendar_entries",
-			"/command/dashcam_save_clip")
+	CommandList["COMMANDS_UNKNOWN"] = []string{
+		"/command/upcoming_calendar_entries",
+		"/command/dashcam_save_clip",
+	}
+
+	// allow all commands available below
+	allowAll := getEnvAsBool("COMMANDS_ALL", false)
+
+	// looping over CommandList to generate allowList
+	for key := range CommandList {
+		// checking if env is set from key or if all should be allowed
+		if getEnvAsBool(key, false) || allowAll {
+			// looping over all values and appending to allowList
+			for _, values := range CommandList[key] {
+				allowList = append(allowList, string(values))
+			}
+		}
 	}

 	// if allowList is empty, read COMMANDS_ALLOWLIST and append to allowList
--- a/src/v1_TeslaMateAPICarsChargesDetails.go
+++ b/src/v1_TeslaMateAPICarsChargesDetails.go
@@ -103,7 +103,7 @@ func TeslaMateAPICarsChargesDetailsV1(c *gin.Context) {
 	}

 	// creating required vars
-	var ChargeData Charge
+	var charge Charge
 	var ChargeDetailsData []ChargeDetails
 	var UnitsLength, UnitsTemperature, CarName string

@@ -151,9 +151,6 @@ func TeslaMateAPICarsChargesDetailsV1(c *gin.Context) {
 	// looping through all results
 	for rows.Next() {

-		// creating charge object based on struct
-		charge := Charge{}
-
 		// scanning row and putting values into the charge
 		err = rows.Scan(
 			&charge.ChargeID,
@@ -199,9 +196,6 @@ func TeslaMateAPICarsChargesDetailsV1(c *gin.Context) {
 			return
 		}

-		// appending charge to ChargeData
-		ChargeData = charge
-
 		// getting detailed charge data from database
 		query = `
 			SELECT
@@ -292,7 +286,7 @@ func TeslaMateAPICarsChargesDetailsV1(c *gin.Context) {

 			// appending drive to ChargeData
 			ChargeDetailsData = append(ChargeDetailsData, chargedetails)
-			ChargeData.ChargeDetails = ChargeDetailsData
+			charge.ChargeDetails = ChargeDetailsData
 		}
 	}

@@ -311,7 +305,7 @@ func TeslaMateAPICarsChargesDetailsV1(c *gin.Context) {
 				CarID:   CarID,
 				CarName: CarName,
 			},
-			Charge: ChargeData,
+			Charge: charge,
 			TeslaMateUnits: TeslaMateUnits{
 				UnitsLength:      UnitsLength,
 				UnitsTemperature: UnitsTemperature,
--- a/src/v1_TeslaMateAPICarsCommand.go
+++ b/src/v1_TeslaMateAPICarsCommand.go
@@ -61,10 +61,9 @@ func TeslaMateAPICarsCommandV1(c *gin.Context) {
 	if command == "/command/" || command == "/command/wake_up" {
 		command = "/wake_up"
 	}
-	log.Println("[debug] TeslaMateAPICarsCommandV1 command received:", command)

 	if !checkArrayContainsString(allowList, command) {
-		log.Print("[warning] TeslaMateAPICarsCommandV1 command: " + command + " not allowed")
+		log.Println("[warning] TeslaMateAPICarsCommandV1 command not allowed!")
 		TeslaMateAPIHandleOtherResponse(c, http.StatusUnauthorized, "TeslaMateAPICarsCommandV1", gin.H{"error": "unauthorized"})
 		return
 	}
--- a/src/v1_TeslaMateAPICarsDrivesDetails.go
+++ b/src/v1_TeslaMateAPICarsDrivesDetails.go
@@ -116,7 +116,7 @@ func TeslaMateAPICarsDrivesDetailsV1(c *gin.Context) {
 	}

 	// creating required vars
-	var DriveData Drive
+	var drive Drive
 	var DriveDetailsData []DriveDetails
 	var UnitsLength, UnitsTemperature, CarName string

@@ -244,9 +244,6 @@ func TeslaMateAPICarsDrivesDetailsV1(c *gin.Context) {
 			return
 		}

-		// appending drive to DriveData
-		DriveData = drive
-
 		// getting detailed drive data from database
 		query = `
 		 			SELECT
@@ -346,9 +343,9 @@ func TeslaMateAPICarsDrivesDetailsV1(c *gin.Context) {
 				return
 			}

-			// appending drive to DriveData
+			// appending drive to drive
 			DriveDetailsData = append(DriveDetailsData, drivedetails)
-			DriveData.DriveDetails = DriveDetailsData
+			drive.DriveDetails = DriveDetailsData
 		}

 		// checking for errors in the rows result
@@ -375,7 +372,7 @@ func TeslaMateAPICarsDrivesDetailsV1(c *gin.Context) {
 				CarID:   CarID,
 				CarName: CarName,
 			},
-			Drive: DriveData,
+			Drive: drive,
 			TeslaMateUnits: TeslaMateUnits{
 				UnitsLength:      UnitsLength,
 				UnitsTemperature: UnitsTemperature,
--- a/src/v1_TeslaMateAPICarsLogging.go
+++ b/src/v1_TeslaMateAPICarsLogging.go
@@ -57,10 +57,9 @@ func TeslaMateAPICarsLoggingV1(c *gin.Context) {

 	// getting :Command
 	command := ("/logging/" + c.Param("Command"))
-	log.Println("[debug] TeslaMateAPICarsLoggingV1 command received:", command)

 	if !checkArrayContainsString(allowList, command) {
-		log.Print("[warning] TeslaMateAPICarsLoggingV1 command: " + command + " not allowed")
+		log.Printf("[warning] TeslaMateAPICarsLoggingV1 command not allowed!")
 		TeslaMateAPIHandleOtherResponse(c, http.StatusUnauthorized, "TeslaMateAPICarsLoggingV1", gin.H{"error": "unauthorized"})
 		return
 	}
--- a/src/v1_TeslaMateAPICarsStatus.go
+++ b/src/v1_TeslaMateAPICarsStatus.go
@@ -178,103 +178,103 @@ func (s *statusCache) newMessage(c mqtt.Client, msg mqtt.Message) {
 	}

 	// running if-else statements to collect data and put into overall vars..
-	if MqttTopic == "display_name" {
+	switch MqttTopic {
+	case "display_name":
 		stat.MQTTDataDisplayName = string(msg.Payload())
-	} else if MqttTopic == "state" {
+	case "state":
 		stat.MQTTDataState = string(msg.Payload())
-	} else if MqttTopic == "since" {
+	case "since":
 		stat.MQTTDataStateSince = string(msg.Payload())
-	} else if MqttTopic == "healthy" {
+	case "healthy":
 		stat.MQTTDataHealthy = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "version" {
+	case "version":
 		stat.MQTTDataVersion = string(msg.Payload())
-	} else if MqttTopic == "update_available" {
+	case "update_available":
 		stat.MQTTDataUpdateAvailable = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "update_version" {
+	case "update_version":
 		stat.MQTTDataUpdateVersion = string(msg.Payload())
-	} else if MqttTopic == "model" {
+	case "model":
 		stat.MQTTDataModel = string(msg.Payload())
-	} else if MqttTopic == "trim_badging" {
+	case "trim_badging":
 		stat.MQTTDataTrimBadging = string(msg.Payload())
-	} else if MqttTopic == "exterior_color" {
+	case "exterior_color":
 		stat.MQTTDataExteriorColor = string(msg.Payload())
-	} else if MqttTopic == "wheel_type" {
+	case "wheel_type":
 		stat.MQTTDataWheelType = string(msg.Payload())
-	} else if MqttTopic == "spoiler_type" {
+	case "spoiler_type":
 		stat.MQTTDataSpoilerType = string(msg.Payload())
-	} else if MqttTopic == "geofence" {
+	case "geofence":
 		stat.MQTTDataGeofence = string(msg.Payload())
-	} else if MqttTopic == "latitude" {
+	case "latitude":
 		stat.MQTTDataLatitude = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "longitude" {
+	case "longitude":
 		stat.MQTTDataLongitude = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "shift_state" {
+	case "shift_state":
 		stat.MQTTDataShiftState = string(msg.Payload())
-	} else if MqttTopic == "power" {
+	case "power":
 		stat.MQTTDataPower = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "speed" {
+	case "speed":
 		stat.MQTTDataSpeed = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "heading" {
+	case "heading":
 		stat.MQTTDataHeading = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "elevation" {
+	case "elevation":
 		stat.MQTTDataElevation = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "locked" {
+	case "locked":
 		stat.MQTTDataLocked = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "sentry_mode" {
+	case "sentry_mode":
 		stat.MQTTDataSentryMode = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "windows_open" {
+	case "windows_open":
 		stat.MQTTDataWindowsOpen = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "doors_open" {
+	case "doors_open":
 		stat.MQTTDataDoorsOpen = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "trunk_open" {
+	case "trunk_open":
 		stat.MQTTDataTrunkOpen = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "frunk_open" {
+	case "frunk_open":
 		stat.MQTTDataFrunkOpen = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "is_user_present" {
+	case "is_user_present":
 		stat.MQTTDataIsUserPresent = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "is_climate_on" {
+	case "is_climate_on":
 		stat.MQTTDataIsClimateOn = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "inside_temp" {
+	case "inside_temp":
 		stat.MQTTDataInsideTemp = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "outside_temp" {
+	case "outside_temp":
 		stat.MQTTDataOutsideTemp = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "is_preconditioning" {
+	case "is_preconditioning":
 		stat.MQTTDataIsPreconditioning = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "odometer" {
+	case "odometer":
 		stat.MQTTDataOdometer = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "est_battery_range_km" {
+	case "est_battery_range_km":
 		stat.MQTTDataEstBatteryRange = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "rated_battery_range_km" {
+	case "rated_battery_range_km":
 		stat.MQTTDataRatedBatteryRange = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "ideal_battery_range_km" {
+	case "ideal_battery_range_km":
 		stat.MQTTDataIdealBatteryRange = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "battery_level" {
+	case "battery_level":
 		stat.MQTTDataBatteryLevel = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "usable_battery_level" {
+	case "usable_battery_level":
 		stat.MQTTDataUsableBatteryLevel = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "plugged_in" {
+	case "plugged_in":
 		stat.MQTTDataPluggedIn = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "charge_energy_added" {
+	case "charge_energy_added":
 		stat.MQTTDataChargeEnergyAdded = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "charge_limit_soc" {
+	case "charge_limit_soc":
 		stat.MQTTDataChargeLimitSoc = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "charge_port_door_open" {
+	case "charge_port_door_open":
 		stat.MQTTDataChargePortDoorOpen = convertStringToBool(string(msg.Payload()))
-	} else if MqttTopic == "charger_actual_current" {
+	case "charger_actual_current":
 		stat.MQTTDataChargerActualCurrent = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "charger_phases" {
+	case "charger_phases":
 		stat.MQTTDataChargerPhases = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "charger_power" {
+	case "charger_power":
 		stat.MQTTDataChargerPower = convertStringToFloat(string(msg.Payload()))
-	} else if MqttTopic == "charger_voltage" {
+	case "charger_voltage":
 		stat.MQTTDataChargerVoltage = convertStringToInteger(string(msg.Payload()))
-	} else if MqttTopic == "scheduled_charging_start_time" {
+	case "scheduled_charging_start_time":
 		stat.MQTTDataScheduledChargingStartTime = string(msg.Payload())
-	} else if MqttTopic == "time_to_full_charge" {
+	case "time_to_full_charge":
 		stat.MQTTDataTimeToFullCharge = convertStringToFloat(string(msg.Payload()))
-	} else {
+	default:
 		log.Printf("[warning] TeslaMateAPICarsStatusV1 mqtt.MessageHandler issue.. extraction of data for %s not implemented!", MqttTopic)
-		return
 	}
 }

--- a/src/webserver.go
+++ b/src/webserver.go
@@ -281,31 +281,31 @@ func getEnvAsInt(name string, defaultVal int) int {

 // convertStringToBool func
 func convertStringToBool(data string) bool {
-	if value, err := strconv.ParseBool(data); err == nil {
+	value, err := strconv.ParseBool(data)
+	if err == nil {
 		return value
 	}
-	// else..
-	log.Println("[warning] convertStringToBool error could not return value (" + data + ") correctly.. returning false")
+	log.Printf("[warning] convertStringToBool error when converting value correctly.. returning false. Error: %s", err)
 	return false
 }

 // convertStringToFloat func
 func convertStringToFloat(data string) float64 {
-	if value, err := strconv.ParseFloat(data, 64); err == nil {
+	value, err := strconv.ParseFloat(data, 64)
+	if err == nil {
 		return value
 	}
-	// else..
-	log.Println("[warning] convertStringToFloat error could not return value (" + data + ") correctly.. returning 0.0")
+	log.Printf("[warning] convertStringToFloat error when converting value correctly.. returning 0.0. Error: %s", err)
 	return 0.0
 }

 // convertStringToInteger func
 func convertStringToInteger(data string) int {
-	if value, err := strconv.Atoi(data); err == nil {
+	value, err := strconv.Atoi(data)
+	if err == nil {
 		return value
 	}
-	// else..
-	log.Println("[warning] convertStringToInteger error could not return value (" + data + ") correctly.. returning 0")
+	log.Printf("[warning] convertStringToInteger error when converting value correctly.. returning 0. Error: %s", err)
 	return 0
 }