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.

Sign up for GitHub

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

Jump to bottom

[http] Improve binding #15376

Closed
wants to merge 10 commits into from
Closed

[http] Improve binding #15376

wants to merge 10 commits into from

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Aug 5, 2023 •
edited
Loading

Closes #13802
Fixes #11920
Fixes #11677
Closes #9265

Depends on openhab/openhab-core#3742

  • allow changing UserAgent
  • add support for TOKEN auth
  • add channels for last success/last failure
  • use core classes for channel handling

@J-N-K J-N-K added enhancement An enhancement or new feature for an existing add-on awaiting other PR Depends on another PR labels Aug 5, 2023
@J-N-K J-N-K requested a review from a team as a code owner August 5, 2023 14:39
@lsiepel lsiepel mentioned this pull request Aug 5, 2023
Closed
@J-N-K J-N-K removed the awaiting other PR Depends on another PR label Aug 11, 2023
@wborn wborn added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Aug 11, 2023
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of an add-on label Aug 18, 2023
@J-N-K
Copy link
Member Author

J-N-K commented Sep 16, 2023

Anything blocking here? I don't want to resolve merge conflicts several times.

@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/oh4-http-binding-cant-be-set-to-pure-writeonly/149623/4

@kaikreuzer kaikreuzer requested a review from wborn September 19, 2023 20:21
jlaur
jlaur requested changes Sep 19, 2023
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks! I have provided some minor feedback from first glance, but would appreciate if @wborn or another @openhab/add-ons-maintainers could have a look also.

bundles/org.openhab.binding.http/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.http/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.http/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.http/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.http/README.md
@@ -141,7 +146,7 @@ Please note that incompatible units (e.g. `°C` for a `Number:Density` item) wil
| `moveValue` | yes | - | A special value that represents `MOVE` |

All values that are not `upValue`, `downValue`, `stopValue`, `moveValue` are interpreted as position 0-100% and need to be numeric only.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

bundles/org.openhab.binding.http/src/main/resources/OH-INF/thing/thing-types.xml Outdated
</config-description>
</thing-type>

<channel-type id="requestDateTime">
<item-type>DateTime</item-type>
<label>Dummy </label>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<label>Dummy </label>
<label>Dummy</label>

I would also suggest to change to a generic label other than "Dummy" since I think it could end up as the actual label in some cases - see openhab/openhab-core#3569.

bundles/org.openhab.binding.http/src/main/resources/OH-INF/update/instructions.xml Show resolved Hide resolved
bundles/org.openhab.binding.http/src/main/resources/OH-INF/update/instructions.xml Show resolved Hide resolved
bundles/org.openhab.binding.http/src/main/resources/OH-INF/update/instructions.xml Show resolved Hide resolved
bundles/org.openhab.binding.http/src/main/resources/OH-INF/update/instructions.xml Show resolved Hide resolved
J-N-K and others added 10 commits October 8, 2023 21:24
@J-N-K
[http] Improve binding
e3ce167 
- allow changing UserAgent
- add support for TOKEN auth
- add channels for last success/last failure
- use core classes for channel handling

Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K
address review comments
4a507f4 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K
fix old authentications not removed when method changes
77f4258 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K
fix PUT/POST requests with stateContent miss proper content-type
bd61ee3 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K
improvement
7d41eaa 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K @jlaur
Update bundles/org.openhab.binding.http/README.md
67725f9 
Co-authored-by: Jacob Laursen <[email protected]>
Signed-off-by: J-N-K <[email protected]>
@J-N-K @jlaur
Update bundles/org.openhab.binding.http/README.md
bfad91b 
Co-authored-by: Jacob Laursen <[email protected]>
Signed-off-by: J-N-K <[email protected]>
@J-N-K @jlaur
Update bundles/org.openhab.binding.http/README.md
2c7672f 
Co-authored-by: Jacob Laursen <[email protected]>
Signed-off-by: J-N-K <[email protected]>
@J-N-K @jlaur
Update bundles/org.openhab.binding.http/README.md
99728fb 
Co-authored-by: Jacob Laursen <[email protected]>
Signed-off-by: J-N-K <[email protected]>
@J-N-K
address review comments
61ca96f 
Signed-off-by: Jan N. Klug <[email protected]>
@J-N-K J-N-K closed this Oct 9, 2023
@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/http-refresh-and-delay/128747/11

@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/amazon-echo-smartj-binding-last-voice-command-not-working-anymore/148296/291

@lsiepel
Copy link
Contributor

lsiepel commented Jan 2, 2024 •
edited
Loading

As this would close several issues, would it be an intermediate solution to have this with the breaking change in the addons repo and have it without the breaking change in smarthomej?

Many breaking changes are about channel/thing id's maybe it would also be nice if the thing upgrade instructions support a update of the id (old => new value), but that is something for core to discuess i guess.

@J-N-K
Copy link
Member Author

J-N-K commented Jan 2, 2024

No.

@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/comparison-to-home-assistant/151931/88

@J-N-K J-N-K mentioned this pull request Jan 14, 2024
Merged
@J-N-K J-N-K deleted the http branch January 15, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur requested changes

@wborn wborn Awaiting requested review from wborn

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
No milestone
5 participants
@J-N-K @openhab-bot @lsiepel @wborn @jlaur