Feature: Adapted structure to HACS integration blueprint

[JanGiese]

Hey,

Thands for creating this integration. I am looking forward to use it in my SmarHome as well, but in order to install it, I have refactored it the HACS integration blueprint structure, so it can be installed from the HACS Custom Repositories. Basically I have moved the Python files to custom_components/fyta_integration and added some descriptive files around it. I have also included the recommended dev-container which can be used for development inside a Docker Container and the GitHub Actions needed for the release into HACS. At last, I have added some docs, so the Linting Action I have added won´t complain anymore.
I hope you´ll find it useful as well. If you have any questions, feel free to reach out to me.

[dontinelli]

Thank you a lot, looks great! I only have two questions before I merge:

1. In the meanwhile I uploaded fyta_cli to Pypi. Would it make sense to add fyta_cli as a requirement instead of delivering it as part of the integration files? I'm not sure what the best practice is in this respect, but it has an influence on the import-lines.
2. With regards to maintenance, can just the files in custom_components/fyta be updated or what else has to be considered for future releases? As you may have noticed, I'm currently working on a PR to include fyta as core integration. However, the review process is rather cumbersome. So while I would like to backport some changes and also further develop the custom integration (which might be easier than bringing improvements to core), I do not have the time for extensive additional work.

[JanGiese]

Thank you a lot, looks great! I only have two questions before I merge:

1. In the meanwhile I uploaded fyta_cli to Pypi. Would it make sense to add fyta_cli as a requirement instead of delivering it as part of the integration files? I'm not sure what the best practice is in this respect, but it has an influence on the import-lines. --> I will test it on the weekend, just send me the link :)
2. With regards to maintenance, can just the files in custom_components/fyta be updated or what else has to be considered for future releases? As you may have noticed, I'm currently working on a PR to include fyta as core integration. However, the review process is rather cumbersome. So while I would like to backport some changes and also further develop the custom integration (which might be easier than bringing improvements to core), I do not have the time for extensive additional work. --> In general, only the custom_components/fyta files need updates for future releases. Dependency updates will be automated via GitHub´s dependabot, which automatically creates PRs.

[dontinelli]

Thank you a lot, looks great! I only have two questions before I merge:

1. In the meanwhile I uploaded fyta_cli to Pypi. Would it make sense to add fyta_cli as a requirement instead of delivering it as part of the integration files? I'm not sure what the best practice is in this respect, but it has an influence on the import-lines. --> I will test it on the weekend, just send me the link :)
2. With regards to maintenance, can just the files in custom_components/fyta be updated or what else has to be considered for future releases? As you may have noticed, I'm currently working on a PR to include fyta as core integration. However, the review process is rather cumbersome. So while I would like to backport some changes and also further develop the custom integration (which might be easier than bringing improvements to core), I do not have the time for extensive additional work. --> In general, only the custom_components/fyta files need updates for future releases. Dependency updates will be automated via GitHub´s dependabot, which automatically creates PRs.

Thank you. If this is ok for you, I will await your test with the requirement (which might need adjustment of the import-lines) before merging.

[dontinelli]

Wow, swift adjustment! fyta_cli also provides fyta_exception and fyta_connector. I would therefore delete these files as well from this repository. Agree?

[JanGiese]

Good Point! But Unfortunately the release does not include the requirement, so when I install it, it can´t find the fyta_cli. At a first glance, I could also not any info about dependencies in the HACS documentation. I will try few more things, cause I also want to know how to use dependencies in custom integrations. Ideas:

1. Include requirements.txt with only fyta_cli in custom_integrations/fyta
2. Refactor release.yml to download fyta_cli and package it with the release artifact

[dontinelli]

Good Point! But Unfortunately the release does not include the requirement, so when I install it, it can´t find the fyta_cli. At a first glance, I could also not any info about dependencies in the HACS documentation. I will try few more things, cause I also want to know how to use dependencies in custom integrations. Ideas:

1. Include requirements.txt with only fyta_cli in custom_integrations/fyta

2. Refactor release.yml to download fyta_cli and package it with the release artifact

Ah ok. Based on your deletion of fyta_client.py I assumed that it worked. Thank you for clarifying!

[dontinelli]

@JanGiese: Based on your commits I assume it now works with including the requirements. Can you please confirm?

[JanGiese]

Hey @dontinelli, I have figured out, that dependencies for custom integrations need to be placed in the manifest.json to get loaded with the integration. Unfortunately this fails for me with the with the current fyta_cli due to empty values in my sensor. To get around that, I have added a safe_get function to the fyta_cli and created PR. So in theory, you can merge this PR, but I was not able to test it end2end. To make a complete test, I would need the above mentioned PR merged and published.

[dontinelli]

Hey @dontinelli, I have figured out, that dependencies for custom integrations need to be placed in the manifest.json to get loaded with the integration. Unfortunately this fails for me with the with the current fyta_cli due to empty values in my sensor. To get around that, I have added a safe_get function to the fyta_cli and created PR. So in theory, you can merge this PR, but I was not able to test it end2end. To make a complete test, I would need the above mentioned PR merged and published.

I merged the PR in fyta_cli and generated a new release (v0.3). Would be good if you could test before I merge and release, it would be a pity if it doesn't work in practice.

[JanGiese]

On it, but I get an exception:

2024-02-28 18:29:16.536 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry jan_giese@gmx.de for fyta
Traceback (most recent call last):
  File "/home/vscode/.local/lib/python3.11/site-packages/homeassistant/config_entries.py", line 406, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/fyta-custom_component/custom_components/fyta/__init__.py", line 36, in async_setup_entry
    fyta = FytaConnector(username, password, access_token, expiration)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/fyta_cli/fyta_connector.py", line 41, in __init__
    self.timezone: ZoneInfo = datetime.timezone.utc if tz == "" else ZoneInfo(tz)
                              ^^^^^^^^^^^^^^^^^
AttributeError: type object 'datetime.datetime' has no attribute 'timezone'

on this line in fyta_connector.py

self.timezone: ZoneInfo = datetime.timezone.utc if tz == "" else ZoneInfo(tz)

[dontinelli]

Hey @dontinelli, I have figured out, that dependencies for custom integrations need to be placed in the manifest.json to get loaded with the integration. Unfortunately this fails for me with the with the current fyta_cli due to empty values in my sensor. To get around that, I have added a safe_get function to the fyta_cli and created PR. So in theory, you can merge this PR, but I was not able to test it end2end. To make a complete test, I would need the above mentioned PR merged and published.

Thank you @JanGiese! I merged PR in fyta_cli and made new release (v0.3).

On it, but I get an exception:

2024-02-28 18:29:16.536 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry jan_giese@gmx.de for fyta
Traceback (most recent call last):
  File "/home/vscode/.local/lib/python3.11/site-packages/homeassistant/config_entries.py", line 406, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/fyta-custom_component/custom_components/fyta/__init__.py", line 36, in async_setup_entry
    fyta = FytaConnector(username, password, access_token, expiration)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vscode/.local/lib/python3.11/site-packages/fyta_cli/fyta_connector.py", line 41, in __init__
    self.timezone: ZoneInfo = datetime.timezone.utc if tz == "" else ZoneInfo(tz)
                              ^^^^^^^^^^^^^^^^^
AttributeError: type object 'datetime.datetime' has no attribute 'timezone'

on this line in fyta_connector.py

self.timezone: ZoneInfo = datetime.timezone.utc if tz == "" else ZoneInfo(tz)

can you adjust the line as follows: self.timezone: ZoneInfo = timezone.utc if tz == "" else ZoneInfo(tz) and add an import from datetime import timezone in your local file to test? If this solves the problem, I would backport this to fyta_cli.

[JanGiese]

Did that, which brings me here:

current_plant |= {
            "last_updated": self.timezone.localize(
                datetime.fromisoformat(plant_data["sensor"]["received_data_at"]))}

with error

AttributeError: 'datetime.timezone' object has no attribute 'localize'

btw. I hate handling timezones 😆

[dontinelli]

Did that, which brings me here:

current_plant |= {
            "last_updated": self.timezone.localize(
                datetime.fromisoformat(plant_data["sensor"]["received_data_at"]))}

with error

AttributeError: 'datetime.timezone' object has no attribute 'localize'

btw. I hate handling timezones 😆

I guess this is due to the reverse of my change you made in your commit (of which I thought you know what you were doing ;-)). Before I had the following line, which I have not tested, though:

        current_plant |= {
            "last_updated": datetime.fromisoformat(
                plant_data["sensor"]["received_data_at"]
            ).astimezone(self.timezone)
        }

[JanGiese]

Btw. is there any way I can reach out to you besides github (slack etc.)? Was fun collaborating with you. (you may want to create a new Tag for the 0.3.0 release in the fyta_cli repo ;))

[dontinelli]

Btw. is there any way I can reach out to you besides github (slack etc.)? Was fun collaborating with you. (you may want to create a new Tag for the 0.3.0 release in the fyta_cli repo ;))

Indeed, it is fun and I really appreciate your collaboration! For the time being I'm only on discord (dontinelli_51643). I guess one can also directly communicate there?