hacs_waste_collection_schedule icon indicating copy to clipboard operation
hacs_waste_collection_schedule copied to clipboard

Add config flow (UI) configuration option

Open dan-r opened this issue 9 months ago • 9 comments

Implements config flow configuration whilst retaining support for YAML configuration. Resolves #2016.

There's some automated testing needed, but I'd appreciate some community testing with this fork as so far I've only tested with a few areas local to me.

Customise functionality is not supported as this time, but the basic functionality (incl. ICS) is there.

This feature borrows very heavily from the config_flow branch, but takes a different approach with how sources are handled. Instead of templating the language and constants files, a JSON list of sources is written by update_docu_links.py and read during the config flow.

https://github.com/mampfes/hacs_waste_collection_schedule/assets/1384852/1fe6ce3f-051d-4151-b5cf-dae5958fbb27

dan-r avatar May 08 '24 09:05 dan-r

  • Seems to work fine I had an error but could not reproduce it again for some reason (logs, see below).
  • I think it would be nice to "move" the wizards to the UI. I thought about a check whether the source or module has a specific method providing values to select.
  • Type checking: what about an optional dictionary containing types of attributes. It might be nice to have to also be able to set vol.Exclusive as we have a lot of sources you can either configure with some kind of ID and with an address (e.g. jumomind_de allows city street house_number or city_id and area_id)
  • low priority and maybe not even wanted but showing the ICS YAMLs in the specific country could lead to the ICS mask with the to-dos shown on top of the configuration fields. Maybe allow to specify default values in the yaml (like regex method or specific data)

General stuff I noticed while looking at the code:

  • why is the first step named user and not country?
  • comment above async_step_source is wrong

one time error log:

  File "/workspaces/core/config/custom_components/waste_collection_schedule/config_flow.py", line 112, in async_step_args
    ] = cv.string if default is None else cv_map[type(default)]
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: TextSelector.__init__() got an unexpected keyword argument 'type'

5ila5 avatar May 10 '24 15:05 5ila5

Thanks for your feedback on these changes!

I think it would be nice to "move" the wizards to the UI. I thought about a check whether the source or module has a specific method providing values to select.

Interesting - I hadn't spotted the sources with wizards. I'll take a look into that.

Type checking: what about an optional dictionary containing types of attributes.

Agreed, I'll look into that.

It might be nice to have to also be able to set vol.Exclusive as we have a lot of sources you can either configure with some kind of ID and with an address (e.g. jumomind_de allows city street house_number or city_id and area_id)

Agreed. I couldn't see a good way to intepret this type of input requirement. It would be a fair bit of initial work but maybe its worth introducing a JSON (or other machine readable) 'definition' of each source?

low priority and maybe not even wanted but showing the ICS YAMLs in the specific country could lead to the ICS mask with the to-dos shown on top of the configuration fields. Maybe allow to specify default values in the yaml (like regex method or specific data)

Seems reasonable - I didn't go any further with ICS because I wanted to keep it as 'generic' as possible. There were a couple of things like request headers that I'd like to add support for in the UI.

why is the first step named user and not country?

'user' is the first step the UI setup wizard calls. See this table. You could have a step called country thats called straight away from the user step though...

comment above async_step_source is wrong

Fixed :)

one time error log:

That looks to be caused by my type 'guessing' logic. I'll see if I can reproduce that.

dan-r avatar May 10 '24 15:05 dan-r

I never used this integration, because the setup process looked complicated to me. I am a newbie to this integration and gave this PR a try.

The waste collection in my city in germany is provided by "app_abfallplus_de". The following problem is not unique to my city, I have tried several others with "app_abfallplus_de".

In the last step of the setup a lot of fields are shown, but only a few are lablled. I dont know what to enter there. Somewhere my street has to be entered, but i dont know where. When i press enter, it tells me, that not all the required fields have been filled in, but these fields are not highlighted. grafik

I want to note, that if i use "Generic" (for ICS) in the next step i am asked for a source, but only ICS is the option. This step can be skipped imo, if "Generic" is selected. grafik

muchcodesuchwow avatar May 13 '24 22:05 muchcodesuchwow

I made some updates https://github.com/dan-r/hacs_waste_collection_schedule/pull/1 @dan-r what do you think? (fixes @muchcodesuchwow issue as well)

5ila5 avatar May 25 '24 15:05 5ila5

@5ila5 thanks for that - merged it in

dan-r avatar Jun 02 '24 21:06 dan-r

I created a new PR where I added some further stuff (https://github.com/dan-r/hacs_waste_collection_schedule/pull/2)

I think we are at a point where we are nearly feature complete compared to the YAML configuration, so we could probably merge this to the master after some people took a look at the code. We would probably need a bit of documentation but this can be done in the mampfes/master repo.

https://github.com/dan-r/hacs_waste_collection_schedule/pull/2#issue-2355841304:

I added a bunch of further configuration options, Not sure if everything I did is a good idea, so an extra set of eyes and feedback would be appreciated.

* allow YAML configuration along GUI configuration (not sure if this implementation is the best idea)

* added Sensor configuration to inital config flow and to the options flow

* added customization configuration to inital config flow and the options flow

* added additional translations support

* added the options for sources to specify parameter translations for different languages (PARAM_TRANSLATIONS parameter)

* added German translations

* added initial support for the static source

* fixed options flow not showing calendar title

Limitations

* You cannot define sensors in the YAML configuration that access the GUI sources (or the other way around)

* You cannot have sensors, that access multiple sources (source_index of YAML configuration) (possible future workaround: the `multiple` wrapper source.

* static source implementation is not ideal and `multiple` sources wrapper is currently not supported

possible future improvement

* implement support for wizards could solve problems with `static` and `multiple` source

5ila5 avatar Jun 16 '24 15:06 5ila5

Any update on this please? thanks

oerix avatar Jun 27 '24 15:06 oerix

Thanks @5ila5 for your work on this, I've merged the changes in. Sorry for not getting round to it sooner! Functionally I think it is ready to go now.

The only part that I think could be a bit confusing to new users is the sensors step (last one). Is it worth adding a 'do you want any sensors' step with potentially one or two pre-made 'examples'? Personally I don't (yet) really use the sensors and just like having the entries added to my calendar.

This could always be done as future work and is just a thought.

Dan

dan-r avatar Jun 28 '24 15:06 dan-r

@mampfes do you want to take a look before merging this?

5ila5 avatar Jun 28 '24 15:06 5ila5

How do I utilise this UI feature? I tried downloading the most recent update and the master download, but I get a message to say the config can only be done in YAML. Am I missing something?

Whizzykid avatar Jul 09 '24 15:07 Whizzykid

It should just be configurable like any other integration in the HA GUI. It sounds like you do not have the master version (HACS might have done something weird?). It will be part of a release soon.

5ila5 avatar Jul 09 '24 15:07 5ila5

A delete, restart and redownload fixed it. Thanks!

Whizzykid avatar Jul 09 '24 15:07 Whizzykid