[netatmo] REFRESH command from BaseThingHandler leads to loads of getstationsdata requests during startup

[aluedt]

In typical applications during the startup/initialization phase lots of channel links are set up. For Netatmo devices this leads to lots of getstationsdata requests, one for every single channel linked.

The problem is the following:
The standard behavior of BaseThingHandler.channelLinked(ChannelUID) is to invoke a REFRESH command for the newly linked channel. This is overridden by AbstractNetatmoThingHandler.channelLinked(ChannelUID), but there a super call is executed first, so REFRESH is also invoked then. When a REFRESH command is handled by
AbstractNetatmoThingHandler.handleCommand(ChannelUID, Command), AbstractNetatmoThingHandler.updateChannels(String) is invoked. For Netatmo devices this is overridden and in NetatmoDeviceHandler.updateChannels(String), where NetatmoDeviceHandler.updateReadings(String) is called, so a full getstationsdata is executed then. As there's lots of channels which are typically linked during startup of applications, this happens very often and may even lead to IPs temporarily blocked by Netatmo and similar.

Expected Behavior

getstationsdata requests should only occur for regular updates (refreshInterval) and when the REFRESH command is explicitly posted, but not when a channel is linked.

Proposed solution:

Current Behavior

getstationsdata is executed repeatedly whenever a channel is linked.

Possible Solution

Simply remove the super call in AbstractNetatmoThingHandler.channelLinked(ChannelUID)

Steps to Reproduce (for Bugs)

Simply set up some channel links and, e.g., set a break point in NetatmoBridgeHandler.getStationsDataBody(String)

Context

Not relevant.

Your Environment

Embedded Busybox linux (or Windows 7 during development), Java 8, latest master

[lolodomo]

@clinique : any thought ?

[lolodomo]

There is already a cache implemented through the refresh strategy.

[lolodomo]

Your initial analysis is correct, we can just add that there is a check whether the data is expired before requesting new data. The problem is that all REFRESH commands are run more or less at the same time and data is considered as expired each time.
My current feeling is that we should add a lock around the update of data to avoid concurrent requests.
I will try to fix that issue.

[lolodomo]

Explanation found. For sthe weather station, the binding tries to identify automatically the refresh delay to consider. For that, it needs two different timestamps. Until that happened, the binding considers data as outdated.
The reason is clear but I have no immediate idea how to fix that.

[Novanic]

Nice, thank you for the fix. :-)