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

[neohub] Recover faster if NeoHub produces empty responses #13889

Merged
merged 3 commits into from
Jan 4, 2023

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Dec 9, 2022

Resolves #13829

Issue

As mentioned in #13829 the bridge sporadically goes offline (due to 'Null or invalid response'), and it remains offline until the next polling cycle produces a valid response, whereupon it goes online again.

Apparently the reason for this is that there is a bug in the NeoHub, whereby it does indeed sporadically produce empty response packages!!

Note: the NeoHub does produce a response so it is technically online, however the response is empty, so the binding has no data to update the channels.

The issue is being discussed on the HeatMiser Developer portal, and it seems the solution (work around) is to simply repeat the command immediately so the NeoHub produces a non empty response.

Furthermore, during testing of this PR I became aware of some inconsistencies between the various methods of the NeoHubSocket resp. NeoHubWebSocket classes..

  • The exception types NeoHubException resp. IOException were used inconsistently between the two classes.
  • The logging messages, formatting, and methodology was different between the two classes.
  • The JSON validity checking was inconsistent between the two classes.

Solution

This PR makes the following changes..

  1. If the hub produces an empty response, instead of waiting for the next polling cycle, the request is repeated immediately.
  2. Use of exception types NeoHubException / IOException is now consistent over NeoHubSocket / NeoHubWebSocket.
  3. Logging messages, formatting, and methodology are now consistent over NeoHubSocket / NeoHubWebSocket.
  4. JSON validity checking is now consistent over NeoHubSocket / NeoHubWebSocket.
  5. Made some improvements to JavaDocs.
  6. Added another JUnit test.

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg added the bug An unexpected problem or unintended behavior of an add-on label Dec 9, 2022
@andrewfg andrewfg requested a review from a team December 9, 2022 14:38
@andrewfg andrewfg self-assigned this Dec 9, 2022
@andrewfg andrewfg marked this pull request as draft December 9, 2022 14:39
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg changed the title [neohub] Binding recovers faster if NeoHub sporadically produces empty responses [neohub] Recover faster when NeoHub sporadically produces empty responses Dec 10, 2022
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg marked this pull request as ready for review December 10, 2022 17:49
@andrewfg andrewfg changed the title [neohub] Recover faster when NeoHub sporadically produces empty responses [neohub] Recover faster if NeoHub sporadically produces empty responses Dec 10, 2022
@andrewfg andrewfg changed the title [neohub] Recover faster if NeoHub sporadically produces empty responses [neohub] Recover faster if NeoHub produces empty responses Dec 11, 2022
@lolodomo
Copy link
Contributor

It looks to me more an enhancement than a bug resolution?

@andrewfg andrewfg added enhancement An enhancement or new feature for an existing add-on and removed bug An unexpected problem or unintended behavior of an add-on labels Dec 12, 2022
@andrewfg
Copy link
Contributor Author

^
Changed tag from 'bug' to 'enhancement'

@andrewfg andrewfg added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Dec 12, 2022
@andrewfg
Copy link
Contributor Author

@lolodomo .. but even if it is 'only' an enhancement it would be nice if it could get into RC1

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 added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 4, 2023
@lolodomo lolodomo merged commit 31fd911 into openhab:main Jan 4, 2023
@lolodomo lolodomo added this to the 4.0 milestone Jan 4, 2023
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
…3889)

* [neohub] resolve issue openhab#13829
* [neohub] harmonise exceptions and logging
* [neohub] improve field name, and log messages

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
…3889)

* [neohub] resolve issue openhab#13829
* [neohub] harmonise exceptions and logging
* [neohub] improve field name, and log messages

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
…3889)

* [neohub] resolve issue openhab#13829
* [neohub] harmonise exceptions and logging
* [neohub] improve field name, and log messages

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg deleted the #13829-neohub branch August 25, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[neohub] Hub goes offline due to 'Null or invalid response'
2 participants