fix: fix url and persistent notification #2398

danielbrunt57 · 2024-08-04T12:40:57Z

Multiple fixes & refactoring, primarily internal URL for proxy and external URL for cloud connections.

for more information, see https://pre-commit.ci

danielbrunt57 · 2024-08-04T12:45:28Z

What have I done wrong?

jleinenbach · 2024-08-04T16:13:11Z

What have I done wrong?

According to ChatGPT:

The automatic adjustments made by black and isort are not errors in the traditional sense but rather indications that the original file did not conform to defined formatting standards. These tools help to standardize and improve the readability of the code. The actual "errors" are violations of these standards, which the tools automatically correct.

Error Analysis:

black:
- black reformatted the file, indicating that the original code did not adhere to PEP 8 standards. This is not a functional error but a formatting inconsistency.
isort:
- isort adjusted the import sorting, meaning the original import order did not follow the conventions. This is also a stylistic correction rather than a functional error.

Specific Changes:

black:

Whitespace and Indentation:
- black adds or removes blank lines to enhance readability.
- It adjusts indentation to ensure structural consistency in the code.

isort:

Import Sorting:
- isort sorts the import statements according to certain rules (e.g., alphabetically) to increase clarity.
- Commented lines and unused imports are removed or reordered.

Why Do These "Errors" Occur?

These "errors" occur because the original code does not meet the stylistic guidelines enforced by black and isort. These tools ensure that the code is consistent and standard-compliant.

Fixing the Issues:

Review the Changes:
- Review the changes made by black and isort to ensure they do not cause any functional issues.
- Test the code after applying the formatting and sorting changes to ensure no unexpected errors were introduced.
Adjust the Settings:
- If certain formatting rules are not desired, the configuration files for black and isort can be adjusted to disable or modify specific rules.

In essence, black and isort are tools for code quality assurance, helping to keep the code consistent and readable. Their application results in stylistic improvements rather than functional errors.

samsonusmc · 2024-08-04T22:16:35Z

As mentioned.. nothing wrong, I saw that the script did some stylistic changes and nothing that affects the logic when I did the review.
You can look at the changes it made by looking at the committer on the pr. 7d081df

alandtse · 2024-08-05T04:08:19Z

If you want to add the fixes I reverted to this so I can squash them in with this, please do.

danielbrunt57 · 2024-08-05T10:53:06Z

If you want to add the fixes I reverted to this so I can squash them in with this, please do.

I'm sorry but I do not understand your statement...

danielbrunt57 · 2024-08-05T10:58:02Z

I just spotted an error in _-init__.py @1403 - I'm missing hass, on 1404...

jleinenbach · 2024-08-05T11:01:08Z

If you want to add the fixes I reverted to this so I can squash them in with this, please do.

I'm sorry but I do not understand your statement...

I don't understand it, too, but I think it has something to do with these reverts:
https://github.com/alandtse/alexa_media_player/pull/2399/files
https://github.com/alandtse/alexa_media_player/pull/2400/files

added 1399: hass,

danielbrunt57 · 2024-08-05T11:25:43Z

add the fixes I reverted to this so I can squash them in with this

I've merged the reverted fixes into my dev and added the missing hass, to async.create_persistent_notification` at line 1326.
You can squash now...

danielbrunt57 · 2024-08-05T11:32:51Z

BTW, I'm up and running fine on 2024.8.0b2 with these changes + alexapy 1.28.1.
I upgraded 2024.7.4 to 2024.8.0b1 & AMP failed to load - partitioned cookie despite my having alexapy 1.28.1
Waited for database upgrade to complete
Removed the config entry & the pickle file & restarted
Re-added the integration and it loaded fine (always does at that point)
Then I spotted 8.0b2 was released 8 minutes ago - installed, restarted.
AMP complained about hass, missing in create persistent notification - added it, restarted and no errors

jleinenbach · 2024-08-05T12:38:31Z

I've merged the reverted fixes into my dev and added the missing hass, to async.create_persistent_notification` at line 1326. You can squash now...

The missing hass, was caused by one of the reverts and I guess the reverts need to be reverted:
https://github.com/alandtse/alexa_media_player/pull/2397/files

robercy · 2024-08-05T14:43:19Z

Following your recipe, It´s working now. Thanks

robercy · 2024-08-05T14:54:04Z

I've merged the reverted fixes into my dev and added the missing hass, to async.create_persistent_notification` at line 1326. You can squash now...

The missing hass, was caused by one of the reverts and I guess the reverts need to be reverted: https://github.com/alandtse/alexa_media_player/pull/2397/files

Correct.

samsonusmc · 2024-08-05T16:10:33Z

Following your recipe, It´s working now. Thanks

can you specify exactly what recipe? I'm running 2024.8.b02, 4.12.4, add hass, to init.py update manifest.json to alexapiy 12.8.1

I did those things but still not working for me.

jleinenbach · 2024-08-05T16:23:43Z

can you specify exactly what recipe?

Sure, just apply these additional changes to the files:
https://github.com/alandtse/alexa_media_player/pull/2397/files

robercy · 2024-08-05T16:40:30Z

Following your recipe, It´s working now. Thanks

can you specify exactly what recipe? I'm running 2024.8.b02, 4.12.4, add hass, to init.py update manifest.json to alexapiy 12.8.1

I did those things but still not working for me.

Hi.
1 - Running core 2024.8.0b2
2 - Upgrade alexapy to 1.28.1
3 - Applying changes on: "init.py" and "config_flow.py", according https://github.com/alandtse/alexa_media_player/pull/2397/files
4 - Reconfiguring integration.
5 - That´s it.

custom_components/alexa_media/__init__.py

Removed code commented during testing

danielbrunt57 · 2024-08-05T20:22:39Z

I just discovered that the previously removed clear_history service somehow remained/returned but without it's key in services.yaml thus triggering an error in the new actions wrapper for services:

Logger: homeassistant.components.script.good_night
Source: helpers/script.py:525
integration: Script (documentation, issues)
First occurred: 5:17:11 AM (4 occurrences)
Last logged: 5:21:10 AM

Good Night: Error executing script. Unexpected error for call_service at pos 7: 'service'
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 525, in _async_step
    await getattr(self, handler)()
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 764, in _async_call_service_step
    response_data = await self._async_run_long_action(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 727, in _async_run_long_action
    return await long_task
           ^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 2763, in async_call
    response_data = await coro
                    ^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 2806, in _execute_service
    return await target(service_call)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/retry/__init__.py", line 555, in async_actions
    _wrap_service_calls(hass, sequence, retry_params)
  File "/config/custom_components/retry/__init__.py", line 493, in _wrap_service_calls
    if action[ATTR_SERVICE] == f"{DOMAIN}.{ACTIONS_SERVICE}":
       ~~~~~~^^^^^^^^^^^^^^
KeyError: 'service'

Fixing...

Removed SERVICE_CLEAR_HISTORY

alandtse · 2024-08-06T06:11:24Z

Can you also identify what you're fixing? The PR title isn't useful for a changelog. Please list the issues it'll close and I can figure out a good way to describe it.

jleinenbach · 2024-08-06T07:05:52Z

@alandtse - As can be seen from our comments above, this PR still lacks the changes you had reverted due to the lack of a proper commit message. Without these changes, not all errors will be fixed:
https://github.com/alandtse/alexa_media_player/pull/2397/files

jleinenbach · 2024-08-06T07:08:04Z

Can you also identify what you're fixing? The PR title isn't useful for a changelog. Please list the issues it'll close and I can figure out a good way to describe it.

This template summary for @danielbrunt57 was created with the help of ChatGPT to save some time:

Changelog

Changes in Version [Version-Number]:

Bug Fixes and Improvements:
- Login Error Fixed: Improved error handling and more specific error messages to address the "Unknown Error Occurred" issue.
- Compatibility with Home Assistant 2024.8.0b0: Updated dependencies and usage of new API methods to ensure compatibility with the latest Home Assistant version.
- Improved URL Handling for Reliability:
  - Different URLs are now used for local and cloud connections. This ensures that the integration functions more reliably across various network scenarios.
Code Refactoring and Cleanup:
- Removal of Deprecated Functions and Services:
  - Removed the clear_history service and other outdated methods to clean up the codebase and improve future maintainability.
- Introduction of New Exceptions:
  - Added specific exception classes (e.g., TimeoutException, LoginInvalidException) for better error handling and diagnosis.
Enhanced Synchronization and Updates:
- Coordinator for Synchronization:
  - Introduced a coordinator for synchronization and regular data updates via async_request_refresh to increase the reliability of the integration.

These changes aim to improve the stability, user experience, and maintainability of the Alexa Media Player integration for Home Assistant.

CONF_HASS_URL missing CONF_URL duplicated in save_user_input_to_config

danielbrunt57 · 2024-08-06T12:52:05Z

Can you also identify what you're fixing? The PR title isn't useful for a changelog. Please list the issues it'll close and I can figure out a good way to describe it.

I believe it addresses all of these issues:
#2402
#2395
#2393
#2392
#2389
#2381
#2379
#2378
#2239

It may also fix:
#2374

BTW, the coordinator wasn't added per se, it was a refactoring to define it once:
910 + coordinator = hass.data[DATA_ALEXAMEDIA]["accounts"][email].get("coordinator")
rather than multiple lines:
931 - coord = hass.data[DATA_ALEXAMEDIA]["accounts"][email]["coordinator"]
1137 - coordinator = hass.data[DATA_ALEXAMEDIA]["accounts"][email].get(
1138 - "coordinator"
1139 - )
1205 - coordinator = hass.data[DATA_ALEXAMEDIA]["accounts"][email].get("coordinator")

Chrisbysalt · 2024-08-06T13:42:24Z

I still have troble to set up AMP.  Here are the steps I took:

I’m running Home Assistant Core 2024.8.0b4.
I upgraded alexapy to version 1.28.1 (hopefully successfully, as I’m using hassos and did it via the terminal).
I applied changes to “init.py” and “config_flow.py” according to this pull request.
I reconfigured the integration.

Despite these steps, after reconfiguring, I get a failure indicating that I need to reauthenticate. After restarting Home Assistant, I see my Echo devices and entities, but they are all unavailable. I already tried an older version and some other things but still it's not working.   Which logs do you need, how do I get that information, and how else can I help to get this running again?

Alexa Media Reauthentication Required
Reauthenticate *** on the Integrations page. Relogin required after 0:00:00.584743 and 1 api calls.

jleinenbach · 2024-08-06T13:58:25Z

I upgraded alexapy to version 1.28.1 (hopefully successfully, as I’m using hassos and did it via the terminal).

You need to edit custom_components\alexa_media\manifest.json and set the alexapy version here:
"requirements": ["alexapy>=1.28.1", "packaging>=20.3", "wrapt>=1.14.0"],

Chrisbysalt · 2024-08-06T14:08:09Z

thank you @jleinenbach but I forgot to mention I also did that.

jleinenbach · 2024-08-06T14:27:51Z

thank you @jleinenbach but I forgot to mention I also did that.

Did you apply the other changes, too, as you just mentioned two files?
https://github.com/alandtse/alexa_media_player/pull/2398/files

I am unsure if simply reconfiguring will suffice, or if it is necessary to start from scratch: delete the configuration, delete the cookie in .storage, and then reboot before creating a new configuration.

Chrisbysalt · 2024-08-06T14:50:32Z

I have now recreated exactly the way you described. When I try to set up the integration again, I get to the forwarding of Amazon Alexa. After I have entered my access data here I am asked how I want to receive my OTP, by phone, sms or otp tool. No matter what I choose I am redirected from different computers back to the Amazon Alexa login page, as soon as I log in again the game starts all over again

jleinenbach · 2024-08-06T15:00:06Z

... I am redirected from different computers back to the Amazon Alexa login page, as soon as I log in again the game starts all over again

I have never observed a login loop like that before. All I can say is that the OTP tool is my default method for Amazon login (not by SMS), and with AMP, I never had another option. Maybe it helps to clear the browser cookies for Amazon.

Chrisbysalt · 2024-08-06T15:02:42Z

oh I apparently had too many otp from amazon, after I deleted everything there and set it up again I was able to complete this step.

However, Alexa threw the error immediately after logging in that I had to log in again. After a restart of HA it initializes forever I hope for the best

Chrisbysalt · 2024-08-06T15:22:13Z

No whatever I do I always get Alexa Media Reauthentication Required

alandtse · 2024-08-06T15:22:53Z

All, the random off topic comments on a PR we're trying to commit are not helpful. Please take any discussions asking for help to an issue.

alandtse · 2024-08-06T15:24:37Z

@alandtse - As can be seen from our comments above, this PR still lacks the changes you had reverted due to the lack of a proper commit message. Without these changes, not all errors will be fixed:
https://github.com/alandtse/alexa_media_player/pull/2397/files

Yah I'm not going to look to deep; that will need to be handled by the community. I will commit whatever is in the PR, but I'm not maintaining this more than handling PR review.

jleinenbach · 2024-08-06T16:12:52Z

Yah I'm not going to look to deep; that will need to be handled by the community. I will commit whatever is in the PR, but I'm not maintaining this more than handling PR review.

I think this is fixed with Daniel's last 2 commits and the other comments suggest that it works although there are other issues which are not caused by this PR.

javiges · 2024-08-06T19:45:31Z

I am new to this, I understand that if I copy only this from init_py :

from homeassistant.components.persistent_notification import ( async_create as async_create_persistent_notification, async_dismiss as async_dismiss_persistent_notification, create as create_persistent_notification, ) from homeassistant.config_entries import SOURCE_IMPORT from homeassistant.const import ( @@ - 1394.6 +1395.7 @@ async def test_login_status(hass, config_entry, login) -> bool: api_calls: int = login.stats.get("api_calls") message += f"Relogin required after {elaspsed_time} and { api_calls} api calls." async_create_persistent_notification( hass,
title="Alexa Media Reauthentication Required", message=message, notification_id=f"alexa_media_{slugify(login.email)}{slugify(login.url[7:])}",

and this from config_flow.py

) from homeassistant import config_entries from homeassistant.components.http.view import HomeAssistantView from homeassistant.components.persistent_notification import ( async_create as async_create_persistent_notification, async_dismiss as async_dismiss_persistent_notification, create as create_persistent_notification, ) from homeassistant.const import CONF_EMAIL, CONF_PASSWORD, CONF_SCAN_INTERVAL, CONF_URL from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult, UnknownFlow @@ -633.7 +638.7 @@ async def test_login(self): "alexa_media_relogin_success", event_data={"email": hide_email(email), "url": login.url}, ) self. hass.components.persistent_notification.async_dismiss( async_dismiss_persistent_notification( f"alexa_media{slugify(email)}{slugify(login.url[7:])}" ) if not self.hass.data[DATA_ALEXAMEDIA]["accounts"].get ( @@ -747,8 +752,8 @@ def _save_user_input_to_config(self, user_input=None) -> None: self.config[CONF_EMAIL] = user_input[CONF_EMAIL] if CONF_PASSWORD in user_input: self.config[CONF_PASSWORD] = user_input[CONF_PASSWORD] if CONF_URL in user_input: self.config[CONF_URL] = user_input[CONF_URL] if CONF_HASS_URL in user_input: self.config[CONF_HASS_URL] = user_input[ CONF_HASS_URL] if CONF_PUBLIC_URL in user_input: self.config[CONF_PUBLIC_URL] = user_input[CONF_PUBLIC_URL] if CONF_SCAN_INTERVAL

in user_input: is it possible for alexa media player to work in 2024.8?, thanks for the project and the help

jleinenbach · 2024-08-06T22:07:31Z

I think we should also change the manifest.json requirements for this PR to:
"requirements": ["alexapy>=1.28.1", "packaging>=20.3", "wrapt>=1.14.0"],

Thanks to @chrisvblemos, we know that the issue with the regional Amazon domains is a problem with alexapy, not AMP, it persists even with version 1.28.1:
#2393 (comment)
https://gitlab.com/keatontaylor/alexapy/-/issues/32

IMO, the current situation is that an alexapy issue remains unresolved with this PR because the manifest.json has not been updated to >=1.28.1. Even then, it will only work for Amazon.com until alexapy fixes the CSRF cookie problem in the next version, which is due to improved Amazon API security.

danielbrunt57 · 2024-08-07T01:05:18Z

I upgraded alexapy to version 1.28.1 (hopefully successfully, as I’m using hassos and did it via the terminal).

It has to be done in the HA container which you can not get to with SSH/Terminal. alexapy does not exist at the level you see via terminal. If you run pip show alexapy and it reports 1.27.10 then you're in the right place to upgrade. If it's not found, keep looking. All of the above is moot though if you manually edit the manifest.json and restart.

I think we should also change the manifest.json requirements for this PR to: "requirements": ["alexapy>=1.28.1", "packaging>=20.3", "wrapt>=1.14.0"],

We can't yet. I tried submitting that a while ago but Alan rejected it stating that more things have to happen regarding a new alexapy release than just the mods that have gone into 1.28.1.

And to reiterate, @alandtse has requested no more comments in this PR unless related directly to this PR. Thanks!

danielbrunt57 and others added 2 commits August 4, 2024 05:40

Sync my latest changes

2eb1c27

Multiple fixes & refactoring, primarily internal URL for proxy and external URL for cloud connections.

style: auto fixes from pre-commit.com hooks

7d081df

for more information, see https://pre-commit.ci

samsonusmc approved these changes Aug 4, 2024

View reviewed changes

danielbrunt57 mentioned this pull request Aug 5, 2024

"message":"Invalid handler specified"} #2381

Closed

jleinenbach mentioned this pull request Aug 5, 2024

Not able to Log In. Unknown Error Occurred #2393

Closed

danielbrunt57 added 2 commits August 5, 2024 04:05

Merge branch 'alandtse:dev' into dev

fcf4799

Update __init__.py

7c83088

added 1399: hass,

alandtse requested changes Aug 5, 2024

View reviewed changes

custom_components/alexa_media/__init__.py Outdated Show resolved Hide resolved

Update __init__.py

893f0ce

Removed code commented during testing

danielbrunt57 added 3 commits August 5, 2024 13:23

Remove lingering clear_history service

25f1db2

services.py: Removed SERVICE_CLEAR_HISTORY

50fcc19

Update const.py

d7ceb1b

Removed SERVICE_CLEAR_HISTORY

jleinenbach mentioned this pull request Aug 6, 2024

Integration can't load with 2024.8.0b0 #2378

Closed

danielbrunt57 added 2 commits August 6, 2024 04:51

Update config_flow.py

08822b0

CONF_HASS_URL missing CONF_URL duplicated in save_user_input_to_config

Update config_flow.py

345dc8c

samsonusmc approved these changes Aug 6, 2024

View reviewed changes

alandtse changed the title ~~fix: multiple changes~~ fix: fix url and persistent notification Aug 7, 2024

alandtse merged commit b700ac7 into alandtse:dev Aug 7, 2024
3 checks passed

jleinenbach mentioned this pull request Aug 7, 2024

failed to SignIn #2407

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fix url and persistent notification #2398

fix: fix url and persistent notification #2398

danielbrunt57 commented Aug 4, 2024

danielbrunt57 commented Aug 4, 2024

jleinenbach commented Aug 4, 2024

samsonusmc commented Aug 4, 2024

alandtse commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024

jleinenbach commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024 •

edited

Loading

jleinenbach commented Aug 5, 2024

robercy commented Aug 5, 2024 •

edited

Loading

robercy commented Aug 5, 2024

samsonusmc commented Aug 5, 2024

jleinenbach commented Aug 5, 2024

robercy commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024 •

edited

Loading

alandtse commented Aug 6, 2024 •

edited

Loading

jleinenbach commented Aug 6, 2024

jleinenbach commented Aug 6, 2024

danielbrunt57 commented Aug 6, 2024 •

edited

Loading

Chrisbysalt commented Aug 6, 2024 •

edited

Loading

jleinenbach commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024 •

edited

Loading

jleinenbach commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024

jleinenbach commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024

alandtse commented Aug 6, 2024

alandtse commented Aug 6, 2024

jleinenbach commented Aug 6, 2024

javiges commented Aug 6, 2024 •

edited

Loading

jleinenbach commented Aug 6, 2024

danielbrunt57 commented Aug 7, 2024

fix: fix url and persistent notification #2398

fix: fix url and persistent notification #2398

Conversation

danielbrunt57 commented Aug 4, 2024

danielbrunt57 commented Aug 4, 2024

jleinenbach commented Aug 4, 2024

Error Analysis:

Specific Changes:

black:

isort:

Why Do These "Errors" Occur?

Fixing the Issues:

samsonusmc commented Aug 4, 2024

alandtse commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024

jleinenbach commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024 • edited Loading

jleinenbach commented Aug 5, 2024

robercy commented Aug 5, 2024 • edited Loading

robercy commented Aug 5, 2024

samsonusmc commented Aug 5, 2024

jleinenbach commented Aug 5, 2024

robercy commented Aug 5, 2024

danielbrunt57 commented Aug 5, 2024 • edited Loading

alandtse commented Aug 6, 2024 • edited Loading

jleinenbach commented Aug 6, 2024

jleinenbach commented Aug 6, 2024

Changelog

Changes in Version [Version-Number]:

danielbrunt57 commented Aug 6, 2024 • edited Loading

Chrisbysalt commented Aug 6, 2024 • edited Loading

jleinenbach commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024 • edited Loading

jleinenbach commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024

jleinenbach commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024

Chrisbysalt commented Aug 6, 2024

alandtse commented Aug 6, 2024

alandtse commented Aug 6, 2024

jleinenbach commented Aug 6, 2024

javiges commented Aug 6, 2024 • edited Loading

jleinenbach commented Aug 6, 2024

danielbrunt57 commented Aug 7, 2024

danielbrunt57 commented Aug 5, 2024 •

edited

Loading

robercy commented Aug 5, 2024 •

edited

Loading

danielbrunt57 commented Aug 5, 2024 •

edited

Loading

alandtse commented Aug 6, 2024 •

edited

Loading

danielbrunt57 commented Aug 6, 2024 •

edited

Loading

Chrisbysalt commented Aug 6, 2024 •

edited

Loading

Chrisbysalt commented Aug 6, 2024 •

edited

Loading

javiges commented Aug 6, 2024 •

edited

Loading