这是indexloc提供的服务，不要输入任何密码
Skip to content

[shelly] Misc changes (small fixes, log improvements, hardened leak prevention on #15922

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
Merged
merged 5 commits into from
Nov 25, 2023
Merged

[shelly] Misc changes (small fixes, log improvements, hardened leak prevention on #15922

merged 5 commits into from
Nov 25, 2023

Conversation

@markus7017
Copy link
Contributor

@markus7017 markus7017 commented Nov 19, 2023 •
edited
Loading

This PR brings some smaller likes and improvements + hardened handling for leak prevention to avoid loosing sockets on initialization/discovery problems.

Closing #15919
Closing #15920

@markus7017
Misc changes (same fixes, log improvements, hardened leak prevention on
3e120b7 
exceptions)

Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017 markus7017 self-assigned this Nov 19, 2023
@markus7017 markus7017 added bug An unexpected problem or unintended behavior of an add-on work in progress A PR that is not yet ready to be merged labels Nov 19, 2023
@markus7017
Copy link
Contributor Author

markus7017 commented Nov 19, 2023

@lolodomo I put this small PR together to include small fixes before adding new features
We could consider to merge the last PR and this one also to 4.0,
otherwise I would at least add support for Shelly Dimmer 10V here to include some more meat
what do you think?

This was linked to issues Nov 19, 2023
[shelly] DOn't initialize from COAP packet when device init failed due to config error #15919
Closed
[shelly] Remove dependency to org.eclipse.jetty.websocket WebSocket server #15920
Closed
@markus7017
typo
bed590d 
Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017 markus7017 removed the work in progress A PR that is not yet ready to be merged label Nov 19, 2023
@markus7017 markus7017 requested a review from lolodomo November 19, 2023 13:55
@markus7017
Copy link
Contributor Author

markus7017 commented Nov 19, 2023

@wborn Could you confirm hat 4.1 has jetty web socket server artifact as part of the core bundle so I could remove the dependency in pom. xml?

lolodomo
lolodomo requested changes Nov 19, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

Review part 1 of 2 (until Shelly2RpcSocket)

@markus7017
review change
233edfe 
Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017
Copy link
Contributor Author

markus7017 commented Nov 19, 2023

@lolodomo changes applied

@wborn
Copy link
Member

wborn commented Nov 19, 2023

Could you confirm hat 4.1 has jetty web socket server artifact as part of the core bundle so I could remove the dependency in pom. xml?

Yes it would be better to remove it so it uses the core dependency at compile time:

https://github.com/openhab/openhab-core/blob/7af02598ef59b4998b19b31b49023b7f54e9cc39/bom/compile/pom.xml#L173-L178

@markus7017
Copy link
Contributor Author

markus7017 commented Nov 20, 2023

Could you confirm hat 4.1 has jetty web socket server artifact as part of the core bundle so I could remove the dependency in pom. xml?

Yes it would be better to remove it so it uses the core dependency at compile time:

https://github.com/openhab/openhab-core/blob/7af02598ef59b4998b19b31b49023b7f54e9cc39/bom/compile/pom.xml#L173-L178

ok, that's what #15920 brings in, just want to double check avoiding unresolved dependency on production systems

@markus7017
typos in shelly.properties; logic error in Shelly2ApiRc.initialize(),
e308bc8 
consolidated API exception handling in base handler (init, command,
status refresh)

Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017
Copy link
Contributor Author

markus7017 commented Nov 22, 2023

Yes it would be better to remove it so it uses the core dependency at compile time:

@wborn Does that also applies to 4.0.5+, so could this PR also go back to 4.0.x without causing trouble with unresolved dependencies?

@lolodomo What do think on merging this and the last PR to 4.0?

@wborn
Copy link
Member

wborn commented Nov 22, 2023

Does that also applies to 4.0.5+, so could this PR also go back to 4.0.x without causing trouble with unresolved dependencies?

I think that should also work.

@markus7017
Copy link
Contributor Author

markus7017 commented Nov 23, 2023

@lolodomo fyi: we did additional testing with this code level and LGTM

lolodomo
lolodomo requested changes Nov 24, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

Final review. Just one small comment

@markus7017
review change
b232b0a 
Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017
Copy link
Contributor Author

markus7017 commented Nov 25, 2023

@lolodomo changes applied

lolodomo
lolodomo approved these changes Nov 25, 2023
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit a13fd80 into openhab:main Nov 25, 2023
@lolodomo lolodomo added this to the 4.1 milestone Nov 25, 2023
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Nov 26, 2023
@markus7017 @andrewfg
[shelly] Misc changes (small fixes, log improvements, hardened leak p…
0b86647 
…revention on (openhab#15922)

* Misc changes (same fixes, log improvements, hardened leak prevention on
exceptions)

---------

Signed-off-by: Markus Michels <markus7017@gmail.com>
@openhab-bot
Copy link
Collaborator

openhab-bot commented Dec 28, 2023

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

https://community.openhab.org/t/shelly-binding/56862/3584

@markus7017 markus7017 deleted the shelly_misc branch January 27, 2024 12:52
@openhab-bot
Copy link
Collaborator

openhab-bot commented Mar 23, 2024

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

https://community.openhab.org/t/shelly-binding/56862/3746

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@markus7017 @austvik
[shelly] Misc changes (small fixes, log improvements, hardened leak p…
112314a 
…revention on (openhab#15922)

* Misc changes (same fixes, log improvements, hardened leak prevention on
exceptions)

---------

Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lolodomo lolodomo lolodomo approved these changes

Assignees

@markus7017 markus7017

Labels

bug An unexpected problem or unintended behavior of an add-on

Projects

None yet

Milestone

4.1

4 participants

@markus7017 @wborn @openhab-bot @lolodomo