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

[darksky] Added warning if parsing of 'location' parameter throws exception #7831

Merged
merged 2 commits into from
Jun 3, 2020

Conversation

cweitkamp
Copy link
Contributor

  • Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
…binding/darksky/internal/handler/DarkSkyWeatherAndForecastHandler.java


Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@@ -136,9 +136,10 @@ public void initialize() {
try {
location = new PointType(config.location);
} catch (IllegalArgumentException e) {
location = null;
logger.warn("Error parsing 'location' parameter: {}", e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

I usually request such logs to be removed, as thing status changes are logged anyway. Why can't we add the message to the thing status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is right. I thought about that too. But why I did not do it was because the error message of the exception is not localized. It is English. The ThingStatus descriptions in this binding are translatable which could look a little bit weird if you have a mixture of languages in it.

I am brainstorming about a solution for localized exception messages in OHC for a while now. Do not know if it is really worth the effort to spend time on it.

Copy link
Member

Choose a reason for hiding this comment

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

I just had a look at the PointType class. As far as I can see there are three cases where an IAE is thrown: empty string, less than two parts, more than three parts, so all are related to the format. What additional information do we get by logging the message? Probably the thing status message shpuld be a bit more clear that the format of location is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How much effort should I spend on a dead binding?

Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

The question is whether we shoulc hange anything at all then...

@J-N-K J-N-K merged commit 9c0755e into openhab:2.5.x Jun 3, 2020
@cweitkamp cweitkamp deleted the feature-darksky-config branch June 3, 2020 11:38
@cpmeister cpmeister added this to the 2.5.6 milestone Jun 3, 2020
@cpmeister cpmeister added the enhancement An enhancement or new feature for an existing add-on label Jun 3, 2020
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: CSchlipp <christian@schlipp.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Daan Meijer <daan@studioseptember.nl>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
…eption (openhab#7831)

* Added warning if parsing of 'location' parameter throws exception

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
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.

3 participants