fix: Setup of integration with generic ICS fails since HA 2025.1

[z-master42]

In my opinion, removes unused and legacy code that prevents the integration from being set up since HA 2025.1 when using a generic ICS file or URL.

See #3259, #3271, #3390, #3397, #3409

[affeldt28]

Why not delete the ICS_v1.py like you mentioned in your comment?

[z-master42]

Yes, you can do this, but it is not really necessary as it is not integrated anywhere and therefore the code is not used.

[schmidtfx]

It is a backwards incompatible fix, is there anyway to set version=1?

[z-master42]

As far as I can read the code, version == 2 is hardcoded.

[ChristophCaina]

I can confirm that this pr has fixed the issue #3271 which I have reported.

Thanks :)

[5ila5]

This breaks existing installations. Just keep the argument

[z-master42]

At what point? "version" is no longer used within the class. This is exactly how it works for me (with an ICS file an some test URL).

[devployment]

attention: I don't know much about python and even less about this code base. But to me it looks like version: int = 2 just declares the default value as version 2. For my limited understanding, the ics datasource configuration can configure version: 1. Shown in the "test comments" above for example.

hacs_waste_collection_schedule/custom_components/waste_collection_schedule/waste_collection_schedule/source/ics.py

Line 37 in b44e14e

"version": 1,

By that, my assumption is that there might be installations out there, that require this code for backwards (version) compatibility. In this class the parameter might not be used any more, but the configuration of these instances seems pretty dynamic.

So, this might be a hack/fix to make it work locally. But probably not a general fix for the problem. But again, not knowing much about this particular code here, just having the same problem with my installation.

[z-master42]

The version can only be selected if WCS is configured via YAML. When using the UI, which is the preferred and default way, it is always version 2. The version only determines which parser is used.

version
(integer) (optional, default: 2)

Selects the underlying ICS file parser:

version: 1 uses recurring_ical_events
version: 2 uses icalevents

For more see configuration via YAML: https://github.com/mampfes/hacs_waste_collection_schedule/blob/master/doc%2Fsource%2Fics.md

[5ila5]

removing this argument breaks every existing configuration using the ics source as the integration tries to set up the source with an argument not accepted by the __init__

The version arguemnt is registered in every ics source configuration. Additionally it can be set in the GUI it just defaults to 2 but you CAN change it:

[z-master42]

Ahh, at the bottom 😉. But it's just the choice of which parser to use and not a compatibility story. If someone uses version 1, it won't work anyway.

[5ila5]

That would still be a problem when removing it from the __init__ as it would throw TypeError: Source.__init__() got an unexpected keyword argument 'version' when setting starting with an existing configuration having the version attribute to anything.

[z-master42]

Here you can see how the implementation was carried out three years ago. ICS_v1.py is the swap of a previous version that has not been used since then.

5ad421b

[z-master42]

https://www.w3schools.com/python/python_conditions.asp 😉

[5ila5]

Just to clarify:

Version is NOT hard coded, it is just a default value.

I opened a PR (#3468) doing what I said above and writing a migration to remove the version argument from the GUI configurations. YAML configuration still using the version argument will see a warning in their logs.

I will merge #3468 if no one finds an issue with that

[Vato9394]

This fix fixed it for me:
https://www.youtube.com/watch?v=yhxbTMrojH0

[z-master42]

Just to clarify:

Version is NOT hard coded, it is just a default value.

I opened a PR (#3468) doing what I said above and writing a migration to remove the version argument from the GUI configurations. YAML configuration still using the version argument will see a warning in their logs.

I will merge #3468 if no one finds an issue with that

I see it. Thank you!

[z-master42]

So can we close this PR here?