Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tellstick] Adopt new API url #16220

Merged
merged 5 commits into from
Jan 7, 2024
Merged

[tellstick] Adopt new API url #16220

merged 5 commits into from
Jan 7, 2024

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Jan 6, 2024

Resolves #15987

Note: not (yet) tested.

Signed-off-by: Andrew Fiddian-Green [email protected]

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Jan 6, 2024
@andrewfg andrewfg requested review from wborn and lsiepel January 6, 2024 12:03
@andrewfg andrewfg self-assigned this Jan 6, 2024
@andrewfg andrewfg requested a review from a team as a code owner January 6, 2024 12:03
@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 6, 2024

Ping @ulfwin @dabj123

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me except for this comment:

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 6, 2024

@andrewfg andrewfg requested a review from wborn January 6, 2024 13:30
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/telldus-live-tellstick-binding-need-new-api/151646/4

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM now 👍

@dabj123
Copy link

dabj123 commented Jan 6, 2024

How about?

  • Do not make more than 10 requests to pa-api.telldus.com per minute.
  • Double-check that your scripts do not attempt to contact the Telldus API again without waiting for at least 10 seconds. Retry without delay will result in HTTP 429 error.

@wborn
Copy link
Member

wborn commented Jan 6, 2024

Maybe you can create a PR for that @dabj123?
It's not easy to implement and test if you do not use Tellstick.

@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 6, 2024

Do not make more than 10 requests to pa-api.telldus.com per minute.

The polling rate of the binding is 10 seconds (so 6 calls per minute) .. so I presume that is Ok.

do not attempt to contact the Telldus API again without waiting for at least 10 seconds
It's not easy to implement and test if you do not use Tellstick.

I agree with @wborn :)

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Can this be cherry picked to 4.1.x?

@lolodomo
Copy link
Contributor

lolodomo commented Jan 7, 2024

Was this change tested by someone and reported as fixing the issues?

@jlaur
Copy link
Contributor

jlaur commented Jan 7, 2024

Can this be cherry picked to 4.1.x?

I don't think so. openhab/openhab-core#3922 was introduced only in 4.2. It would have been better with separate PR's for the URL bugfix and the USB suggestion enhancement. @andrewfg - will you be able to split the PR on short notice in order to possible get the URL fix into 4.1.1?

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg changed the title [tellstick] Adopt new API url. Add support for suggestion finder. [tellstick] Adopt new API url Jan 7, 2024
@jlaur jlaur removed the enhancement An enhancement or new feature for an existing add-on label Jan 7, 2024
@jlaur
Copy link
Contributor

jlaur commented Jan 7, 2024

Was this change tested by someone and reported as fixing the issues?

Ping @dabj123, @ulfwin

@ulfwin
Copy link
Contributor

ulfwin commented Jan 7, 2024

I actually bought another base station using the local API as a work around, rather than Telldus live. If @dabj123 still uses live, it's probably easier to test for him.

@jlaur
Copy link
Contributor

jlaur commented Jan 7, 2024

I checked https://telldus.com/driftinformation/ which is in Swedish (which is readable for me as Danish 🙂):

Om du använder våra API-tjänster, vänligen notera följande:

  • Använd INTE api.telldus.com. Din IP kommer automatiskt att blockeras. Byt till pa-api.telldus.com om du inte redan har gjort det.
  • Gör inte mer än 10 förfrågningar till pa-api.telldus.com per minut.
  • Kontrollera och dubbelkolla att dina skript inte gör nya försök att kontakta Telldus API utan att vänta minst 10 sekunder. Försök på nytt utan fördröjning kommer innebära HTTP 429 error.

The first bullet "Använd INTE api.telldus.com. Din IP kommer automatiskt att blockeras." means: "do NOT use api.telldus.com. Your IP will automatically be blocked." So we can safely merge as it's pretty clear the old endpoint cannot be used anymore.

@jlaur jlaur merged commit 9a4c556 into openhab:main Jan 7, 2024
3 checks passed
@jlaur jlaur added this to the 4.2 milestone Jan 7, 2024
jlaur pushed a commit that referenced this pull request Jan 7, 2024
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@jlaur jlaur added the patch A PR that has been cherry-picked to a patch release branch label Jan 7, 2024
@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 7, 2024

which is in Swedish (which is readable for me as Danish

@jlaur yes I could read it too .. via Google Translate ;)

@dabj123
Copy link

dabj123 commented Jan 7, 2024

I still use Telldus live bridge, but it seams to not work for me with new jar "org.openhab.binding.tellstick-4.2.0-SNAPSHOT.jar"

Request [http://pa-api.telldus.com/xml/devices/list?supportedMethods=19] failed: ExecutionException HTTP request returned status code 401
2024-01-07 15:06:53.774 [WARN ] [ternal.live.TelldusLiveBridgeHandler] - Failed to update
org.openhab.binding.tellstick.internal.live.TelldusLiveException: HTTP request returned status code 401
at org.openhab.binding.tellstick.internal.live.TelldusLiveDeviceController.callRestMethod(TelldusLiveDeviceController.java:299) ~[?:?]
at org.openhab.binding.tellstick.internal.live.TelldusLiveBridgeHandler.updateDevices(TelldusLiveBridgeHandler.java:155) ~[?:?]
at org.openhab.binding.tellstick.internal.live.TelldusLiveBridgeHandler.refreshDeviceList(TelldusLiveBridgeHandler.java:124) ~[?:?]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
at java.lang.Thread.run(Thread.java:833) [?:?]
Caused by: java.util.concurrent.ExecutionException: HTTP request returned status code 401
at org.openhab.binding.tellstick.internal.live.TelldusLiveDeviceController.innerCallRest(TelldusLiveDeviceController.java:350) ~[?:?]
at org.openhab.binding.tellstick.internal.live.TelldusLiveDeviceController.callRestMethod(TelldusLiveDeviceController.java:290) ~[?:?]
... 8 more

@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 7, 2024

it seams to not work for me with new jar

@dabj123 an HTTP 401 error means 'authentication failed' so check your login credentials please.

@dabj123
Copy link

dabj123 commented Jan 7, 2024

I know that, but I have double check every thing and generate new keys, but still recieve 401 or 429.

@lsiepel
Copy link
Contributor

lsiepel commented Jan 7, 2024

I know that, but I have double check every thing and generate new keys, but still recieve 401 or 429.

Could you open a new issue to investigate that?

@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 7, 2024

but still recieve 401 or 429

@dabj123 these are TWO different issues..

  • HTTP 401 is an authentication error
  • HTTP 429 is because you are sending a retry request faster than 10 seconds "Kontrollera och dubbelkolla att dina skript inte gör nya försök att kontakta Telldus API utan att vänta minst 10 sekunder. Försök på nytt utan fördröjning kommer innebära HTTP 429 error."

I am guessing that if the HTTP 401 error is fixed, then there will not be any retries, so the HTTP 429 error will probably go away.

@dabj123
Copy link

dabj123 commented Jan 7, 2024

Request [http://pa-api.telldus.com/xml/devices/list?supportedMethods=19] failed: ExecutionException HTTP request returned status code 401

OAuth Verification Failed: Can't verify request, missing oauth_consumer_key or oauth_token

@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 7, 2024

OAuth Verification Failed: Can't verify request, missing oauth_consumer_key or oauth_token

@dabj123 you are the TellStick user (apparently the only one) so only you know how to renew your user key or token. I cannot help you since I do not know the binding or the application.

@dabj123
Copy link

dabj123 commented Jan 7, 2024

I have renew new keys etc. Maybe TellStick have new logic in the new api to handle OAuth Verification...

@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 7, 2024

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
@andrewfg andrewfg deleted the tellstick branch August 25, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tellstick] Binding doesn´t work any more. Have to change api url to pa-api.telldus.com
8 participants