From 5662d0e55b4d33d6b34640233cd04e4d5e46de4e Mon Sep 17 00:00:00 2001 From: andig Date: Fri, 22 Jul 2022 10:51:58 +0200 Subject: [PATCH] Fix defaults not applied and improve testability (#3777) --- cmd/configure.go | 8 ++++- cmd/configure/flow.go | 8 ++--- cmd/configure/helper.go | 22 ++++++++++--- cmd/configure/localization/de.toml | 4 +-- cmd/configure/main.go | 46 ++++++++++++++++---------- util/templates/init.go | 52 ++++++++++++++++++------------ util/templates/template.go | 10 ++++++ util/templates/template_modbus.go | 11 +++++-- 8 files changed, 109 insertions(+), 52 deletions(-) diff --git a/cmd/configure.go b/cmd/configure.go index 7ef88fa21..5772cf392 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -25,6 +25,7 @@ func init() { configureCmd.Flags().String("lang", "", "Define the localization to be used (en, de)") configureCmd.Flags().Bool("advanced", false, "Enables handling of advanced configuration options") configureCmd.Flags().Bool("expand", false, "Enables rendering expanded configuration files") + configureCmd.Flags().String("category", "", "Pre-select device category for advanced configuration (implies advanced)") } func runConfigure(cmd *cobra.Command, args []string) { @@ -45,6 +46,11 @@ func runConfigure(cmd *cobra.Command, args []string) { panic(err) } + category, err := cmd.Flags().GetString("category") + if err != nil { + panic(err) + } + util.LogLevel(viper.GetString("log"), nil) stopC := make(chan struct{}) @@ -63,7 +69,7 @@ func runConfigure(cmd *cobra.Command, args []string) { os.Exit(1) }() - impl.Run(log, lang, advanced, expand) + impl.Run(log, lang, advanced, expand, category) close(stopC) <-shutdown.Done() diff --git a/cmd/configure/flow.go b/cmd/configure/flow.go index 5616c0b0a..cdbd1cb9b 100644 --- a/cmd/configure/flow.go +++ b/cmd/configure/flow.go @@ -21,7 +21,7 @@ func (c *CmdConfigure) configureDeviceGuidedSetup() { deviceItem := device{} - for ok := true; ok; { + for { fmt.Println() templateItem, err = c.processDeviceSelection(DeviceCategoryGuidedSetup) @@ -135,7 +135,7 @@ func (c *CmdConfigure) configureLinkedTypes(templateItem templates.Template) { continue } - for ok := true; ok; { + for { if added := c.configureLinkedTemplate(linkedTemplateItem, category); added { deviceOfTemplateAdded[linkedTemplate.Template] = true } @@ -155,7 +155,7 @@ func (c *CmdConfigure) configureLinkedTypes(templateItem templates.Template) { // configureLinkedTemplate lets the user configure a device that is marked as being linked to a guided device // returns true if a device was added func (c *CmdConfigure) configureLinkedTemplate(templateItem templates.Template, category DeviceCategory) bool { - for ok := true; ok; { + for { deviceItem := device{} values := c.processConfig(&templateItem, category) @@ -196,7 +196,7 @@ func (c *CmdConfigure) configureDeviceCategory(deviceCategory DeviceCategory) (d var capabilities []string // repeat until the device is added or the user chooses to continue without adding a device - for ok := true; ok; { + for { fmt.Println() templateItem, err := c.processDeviceSelection(deviceCategory) diff --git a/cmd/configure/helper.go b/cmd/configure/helper.go index 640155271..4c9b4d993 100644 --- a/cmd/configure/helper.go +++ b/cmd/configure/helper.go @@ -265,7 +265,7 @@ func (c *CmdConfigure) configureMQTT(templateItem templates.Template) (map[strin var err error - for ok := true; ok; { + for { fmt.Println() _, paramHost := templateItem.ConfigDefaults.ParamByName("host") _, paramPort := templateItem.ConfigDefaults.ParamByName("port") @@ -318,8 +318,6 @@ func (c *CmdConfigure) configureMQTT(templateItem templates.Template) (map[strin return nil, fmt.Errorf("failed configuring mqtt: %w", err) } } - - return nil, fmt.Errorf("failed configuring mqtt: %w", err) } // fetchElements returns template items of a given class @@ -405,6 +403,19 @@ func (c *CmdConfigure) processConfig(templateItem *templates.Template, deviceCat c.processModbusConfig(templateItem, deviceCategory) + // TODO remove + // type mapped = struct { + // Name string + // Default any + // } + + // fmt.Printf("%+v\n", lo.Map(templateItem.Params, func(p templates.Param, _ int) mapped { + // return mapped{ + // Name: p.Name, + // Default: p.Default, + // } + // })) + return c.processParams(templateItem, deviceCategory) } @@ -500,7 +511,7 @@ func (c *CmdConfigure) processListInputConfig(param templates.Param) []string { var values []string // ask for values until the user decides to stop - for ok := true; ok; { + for { newValue := c.processInputConfig(param) values = append(values, newValue) @@ -549,7 +560,8 @@ func (c *CmdConfigure) processInputConfig(param templates.Param) string { return value } -// handle user input for a device modbus configuration +// processModbusConfig adds default values from the modbus Param to the template +// and handles user input for interface type selection func (c *CmdConfigure) processModbusConfig(templateItem *templates.Template, deviceCategory DeviceCategory) { var choices []string var choiceTypes []string diff --git a/cmd/configure/localization/de.toml b/cmd/configure/localization/de.toml index faf91c207..29f1430fb 100644 --- a/cmd/configure/localization/de.toml +++ b/cmd/configure/localization/de.toml @@ -1,4 +1,4 @@ -Intro = "Die nächsten Schritte führen durch die Einrichtung einer Konfigurationsdatei für evcc.\nBeachte dass dieser Prozess nicht alle möglichen Szenarien berücksichtigen kann.\nDurch Drücken von CTRL-C kann der Prozess abgebrochen werden.\n\nACHTUNG: Diese Funktionalität hat experimentellen Status!\n D.h. es kann möglich sein, dass die hiermit erstellte Konfigurationsdatei\n in einem Update nicht mehr funktionieren könnte und neu erzeugt werden müsste.\n Wir freuen uns auf euer Feedback auf https://github.com/evcc-io/evcc/discussions/\n\nAuf geht`s:" +Intro = "Die nächsten Schritte führen durch die Einrichtung einer Konfigurationsdatei für evcc.\nBeachte dass dieser Prozess nicht alle möglichen Szenarien berücksichtigen kann.\nDurch Drücken von CTRL-C kann der Prozess abgebrochen werden.\n\nACHTUNG: Diese Funktionalität hat experimentellen Status!\n D.h. es kann möglich sein, dass die hiermit erstellte Konfigurationsdatei\n in einem Update nicht mehr funktionieren könnte und neu erzeugt werden müsste.\n Wir freuen uns auf euer Feedback auf https://github.com/evcc-io/evcc/discussions/\n\nAuf geht's:" Flow_Mode = "In welchem Modus soll die Konfiguration durchgeführt werden?" Flow_Mode_Standard = "Standard Modus (So einfach und schnell wie möglich)" @@ -12,7 +12,7 @@ Flow_NewConfiguration_Setup = "- Hausinstallation einrichten" Flow_NewConfiguration_Select = "Wähle eines der folgenden PV Komplettsysteme aus, oder '{{ .ItemNotPresent }}' falls keines dieser Geräte vorhanden ist" Flow_SingleDevice_Setup = "- Ein Gerät konfigurieren" -Flow_SingleDevice_Select = "Wähle eines der folgenden Gerätekategorien aus" +Flow_SingleDevice_Select = "Wähle eine der folgenden Gerätekategorien aus:" Flow_SingleDevice_Config = "Die Konfiguration lautet:" Flow_SMAHems_Setup = "- SMA HEMS konfigurieren" diff --git a/cmd/configure/main.go b/cmd/configure/main.go index 987112d10..0dc74caee 100644 --- a/cmd/configure/main.go +++ b/cmd/configure/main.go @@ -40,7 +40,7 @@ type CmdConfigure struct { } // Run starts the interactive configuration -func (c *CmdConfigure) Run(log *util.Logger, flagLang string, advancedMode, expandedMode bool) { +func (c *CmdConfigure) Run(log *util.Logger, flagLang string, advancedMode, expandedMode bool, category string) { c.log = log c.advancedMode = advancedMode c.expandedMode = expandedMode @@ -72,7 +72,7 @@ func (c *CmdConfigure) Run(log *util.Logger, flagLang string, advancedMode, expa fmt.Println() fmt.Println(c.localizedString("Intro", nil)) - if !c.advancedMode { + if !c.advancedMode && category == "" { // ask the user for his knowledge, so advanced mode can also be turned on this way fmt.Println() flowIndex, _ := c.askChoice(c.localizedString("Flow_Mode", nil), []string{ @@ -84,11 +84,22 @@ func (c *CmdConfigure) Run(log *util.Logger, flagLang string, advancedMode, expa } } - if !c.advancedMode { + if !c.advancedMode && category == "" { c.flowNewConfigFile() return } + if category != "" { + for cat := range DeviceCategories { + if cat == DeviceCategory(category) { + c.flowSingleDevice(DeviceCategory(category)) + return + } + } + + panic("invalid category: " + category) + } + fmt.Println() flowIndex, _ := c.askChoice(c.localizedString("Flow_Type", nil), []string{ c.localizedString("Flow_Type_NewConfiguration", nil), @@ -98,12 +109,12 @@ func (c *CmdConfigure) Run(log *util.Logger, flagLang string, advancedMode, expa case 0: c.flowNewConfigFile() case 1: - c.flowSingleDevice() + c.flowSingleDevice("") } } // configureSingleDevice implements the flow for getting a single device configuration -func (c *CmdConfigure) flowSingleDevice() { +func (c *CmdConfigure) flowSingleDevice(category DeviceCategory) { fmt.Println() fmt.Println(c.localizedString("Flow_SingleDevice_Setup", nil)) fmt.Println() @@ -119,18 +130,19 @@ func (c *CmdConfigure) flowSingleDevice() { DeviceCategories[DeviceCategoryVehicle].title, } - fmt.Println() - _, cagetoryTitle := c.askChoice(c.localizedString("Flow_SingleDevice_Select", nil), categoryChoices) + if category == "" { + fmt.Println() + _, categoryTitle := c.askChoice(c.localizedString("Flow_SingleDevice_Select", nil), categoryChoices) - var selectedCategory DeviceCategory - for item, data := range DeviceCategories { - if data.title == cagetoryTitle { - selectedCategory = item - break + for item, data := range DeviceCategories { + if data.title == categoryTitle { + category = item + break + } } } - devices := c.configureDevices(selectedCategory, false, false) + devices := c.configureDevices(category, false, false) for _, item := range devices { fmt.Println() fmt.Println(c.localizedString("Flow_SingleDevice_Config", localizeMap{})) @@ -178,8 +190,8 @@ func (c *CmdConfigure) flowNewConfigFile() { filename := DefaultConfigFilename - for ok := true; ok; { - file, err := os.OpenFile(filename, os.O_WRONLY, 0o666) + for { + file, err := os.OpenFile(filename, os.O_WRONLY, 0666) if errors.Is(err, os.ErrNotExist) { break } @@ -233,7 +245,7 @@ func (c *CmdConfigure) configureDevices(deviceCategory DeviceCategory, askAdding } } - for ok := true; ok; { + for { device, capabilities, err := c.configureDeviceCategory(deviceCategory) if err != nil { break @@ -279,7 +291,7 @@ func (c *CmdConfigure) configureLoadpoints() { fmt.Println() fmt.Println(c.localizedString("Loadpoint_Setup", nil)) - for ok := true; ok; { + for { loadpointTitle := c.askValue(question{ label: c.localizedString("Loadpoint_Title", nil), diff --git a/util/templates/init.go b/util/templates/init.go index 45ab8d282..8450cc894 100644 --- a/util/templates/init.go +++ b/util/templates/init.go @@ -21,9 +21,36 @@ const ( Vehicle = "vehicle" ) -func loadTemplates(class string) { +func init() { configDefaults.LoadDefaults() +} +func FromBytes(b []byte) (Template, error) { + var definition TemplateDefinition + if err := yaml.Unmarshal(b, &definition); err != nil { + return Template{}, err + } + + tmpl := Template{ + TemplateDefinition: definition, + ConfigDefaults: configDefaults, + } + + err := tmpl.ResolvePresets() + if err == nil { + err = tmpl.ResolveGroup() + } + if err == nil { + err = tmpl.UpdateParamsWithDefaults() + } + if err == nil { + err = tmpl.Validate() + } + + return tmpl, err +} + +func loadTemplates(class string) { if templates[class] != nil { return } @@ -41,26 +68,9 @@ func loadTemplates(class string) { return err } - var definition TemplateDefinition - if err = yaml.Unmarshal(b, &definition); err != nil { - return fmt.Errorf("reading template '%s' failed: %w", filepath, err) - } - - tmpl := Template{ - TemplateDefinition: definition, - ConfigDefaults: configDefaults, - } - if err = tmpl.ResolvePresets(); err != nil { - return err - } - if err = tmpl.ResolveGroup(); err != nil { - return err - } - if err = tmpl.UpdateParamsWithDefaults(); err != nil { - return err - } - if err = tmpl.Validate(); err != nil { - return err + tmpl, err := FromBytes(b) + if err != nil { + return fmt.Errorf("processing template '%s' failed: %w", filepath, err) } path := path.Dir(filepath) diff --git a/util/templates/template.go b/util/templates/template.go index bafa62324..0de3071ca 100644 --- a/util/templates/template.go +++ b/util/templates/template.go @@ -181,6 +181,16 @@ func (t *Template) Defaults(renderMode string) map[string]interface{} { return values } +// SetParamDefault updates the default value of a param +func (t *Template) SetParamDefault(name string, value string) { + for i, p := range t.Params { + if p.Name == name { + t.Params[i].Default = value + return + } + } +} + // return the param with the given name func (t *Template) ParamByName(name string) (int, Param) { for i, p := range t.Params { diff --git a/util/templates/template_modbus.go b/util/templates/template_modbus.go index 7f66bd7c5..b4303c272 100644 --- a/util/templates/template_modbus.go +++ b/util/templates/template_modbus.go @@ -9,7 +9,7 @@ import ( //go:embed modbus.tpl var modbusTmpl string -// add the modbus params to the template +// ModbusParams adds the modbus parameters' default values func (t *Template) ModbusParams(modbusType string, values map[string]interface{}) { if len(t.ModbusChoices()) == 0 { return @@ -92,7 +92,14 @@ func (t *Template) ModbusValues(renderMode string, values map[string]interface{} } if defaultValue != "" { - values[p.Name] = defaultValue + // for modbus params the default value is carried + // using the parameter default, not the value + // TODO figure out why that's necessary + if renderMode == TemplateRenderModeInstance { + t.SetParamDefault(p.Name, defaultValue) + } else { + values[p.Name] = defaultValue + } } }