Skip to content

Commit 580f7b3

Browse files
jimtngjimtng
authored
[fronius] Replace global request lock with per‑bridge locking and non‑blocking polling (openhab#20378)
* [fronius] Add per-host request locks and non-blocking polling Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
1 parent 6abb6d0 commit 580f7b3

7 files changed

Lines changed: 400 additions & 32 deletions

File tree

‎bundles/org.openhab.binding.fronius/src/main/java/org/openhab/binding/fronius/internal/api/FroniusBatteryControl.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public class FroniusBatteryControl {
6464

6565
private final Logger logger = LoggerFactory.getLogger(FroniusBatteryControl.class);
6666
private final Gson gson = new Gson();
67+
private final FroniusHttpUtil httpUtil;
6768
private final HttpClient httpClient;
6869
private final SemverVersion firmwareVersion;
6970
private final URI baseUri;
@@ -74,16 +75,18 @@ public class FroniusBatteryControl {
7475

7576
/**
7677
* Creates a new instance of {@link FroniusBatteryControl}.
77-
*
78+
*
79+
* @param httpUtil the HTTP utility to use for bridge-scoped request coordination
7880
* @param httpClient the HTTP client to use
7981
* @param firmwareVersion the firmware version of the inverter
8082
* @param scheme http or https
8183
* @param hostname the hostname or IP address of the inverter
8284
* @param username the username for the inverter Web UI
8385
* @param password the password for the inverter Web UI
8486
*/
85-
public FroniusBatteryControl(HttpClient httpClient, SemverVersion firmwareVersion, String scheme, String hostname,
86-
String username, String password) {
87+
public FroniusBatteryControl(FroniusHttpUtil httpUtil, HttpClient httpClient, SemverVersion firmwareVersion,
88+
String scheme, String hostname, String username, String password) {
89+
this.httpUtil = httpUtil;
8790
this.httpClient = httpClient;
8891
this.firmwareVersion = firmwareVersion;
8992
this.baseUri = getBaseUri(firmwareVersion, scheme, hostname);
@@ -115,7 +118,7 @@ private TimeOfUseRecords getTimeOfUse() throws FroniusCommunicationException, Fr
115118
Properties headers = new Properties();
116119
headers.put(HttpHeader.AUTHORIZATION.asString(), authHeader);
117120
// Get the time of use settings
118-
String response = FroniusHttpUtil.executeUrl(HttpMethod.GET, timeOfUseUri.toString(), headers, null, null,
121+
String response = httpUtil.executeUrl(HttpMethod.GET, timeOfUseUri.toString(), headers, null, null,
119122
API_TIMEOUT);
120123
logger.trace("Time of Use settings read successfully");
121124

@@ -149,7 +152,7 @@ private void setTimeOfUse(TimeOfUseRecords records)
149152

150153
// Set the time of use settings
151154
String json = gson.toJson(records);
152-
String responseString = FroniusHttpUtil.executeUrl(HttpMethod.POST, timeOfUseUri.toString(), headers,
155+
String responseString = httpUtil.executeUrl(HttpMethod.POST, timeOfUseUri.toString(), headers,
153156
new ByteArrayInputStream(json.getBytes()), "application/json", API_TIMEOUT);
154157
@Nullable
155158
PostConfigResponse response = gson.fromJson(responseString, PostConfigResponse.class);
@@ -162,7 +165,7 @@ private void setTimeOfUse(TimeOfUseRecords records)
162165

163166
/**
164167
* Adds a schedule to the time of use settings of the Fronius hybrid inverter.
165-
*
168+
*
166169
* @param from start time of the forced charge period
167170
* @param until end time of the forced charge period
168171
* @param scheduleType the type of the schedule
@@ -254,7 +257,7 @@ public void addForcedBatteryChargingSchedule(LocalTime from, LocalTime until, Qu
254257

255258
/**
256259
* Prevents the battery from charging right now.
257-
*
260+
*
258261
* @throws FroniusCommunicationException when an error occurs during communication with the inverter
259262
* @throws FroniusUnauthorizedException when the login fails due to invalid credentials
260263
*/
@@ -265,7 +268,7 @@ public void preventBatteryCharging() throws FroniusCommunicationException, Froni
265268

266269
/**
267270
* Prevents the battery from charging during a specific time period.
268-
*
271+
*
269272
* @param from start time of the prevented charging period
270273
* @param until end time of the prevented charging period
271274
* @throws FroniusCommunicationException when an error occurs during communication with the inverter
@@ -278,7 +281,7 @@ public void addPreventBatteryChargingSchedule(LocalTime from, LocalTime until)
278281

279282
/**
280283
* Forces the battery to discharge right now with the specified power.
281-
*
284+
*
282285
* @param power the power to discharge the battery with
283286
* @throws FroniusCommunicationException when an error occurs during communication with the inverter
284287
* @throws FroniusUnauthorizedException when the login fails due to invalid credentials
@@ -291,7 +294,7 @@ public void forceBatteryDischarging(QuantityType<Power> power)
291294

292295
/**
293296
* Forces the battery to discharge during a specific time period with the specified power.
294-
*
297+
*
295298
* @param from start time of the prevented charging period
296299
* @param until end time of the prevented charging period
297300
* @param power the power to discharge the battery with
@@ -325,7 +328,7 @@ public void setBackupReservedCapacity(int percent)
325328

326329
// Set the setting
327330
String json = gson.toJson(Map.of(BACKUP_RESERVED_CAPACITY_PARAMETER, percent));
328-
String responseString = FroniusHttpUtil.executeUrl(HttpMethod.POST, batteriesUri.toString(), headers,
331+
String responseString = httpUtil.executeUrl(HttpMethod.POST, batteriesUri.toString(), headers,
329332
new ByteArrayInputStream(json.getBytes()), "application/json", API_TIMEOUT);
330333
@Nullable
331334
PostConfigResponse response = gson.fromJson(responseString, PostConfigResponse.class);

‎bundles/org.openhab.binding.fronius/src/main/java/org/openhab/binding/fronius/internal/api/FroniusHttpUtil.java‎

Lines changed: 96 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.io.IOException;
1616
import java.io.InputStream;
1717
import java.util.Properties;
18+
import java.util.concurrent.locks.ReentrantLock;
1819

1920
import org.eclipse.jdt.annotation.NonNullByDefault;
2021
import org.eclipse.jdt.annotation.Nullable;
@@ -31,6 +32,19 @@
3132
@NonNullByDefault
3233
public class FroniusHttpUtil {
3334
private static final Logger LOGGER = LoggerFactory.getLogger(FroniusHttpUtil.class);
35+
private final ReentrantLock requestLock = new ReentrantLock();
36+
37+
private enum RequestMode {
38+
CONTROL,
39+
POLLING
40+
}
41+
42+
@FunctionalInterface
43+
interface RequestExecutor {
44+
@Nullable
45+
String execute(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
46+
@Nullable InputStream content, @Nullable String contentType, int timeout) throws IOException;
47+
}
3448

3549
/**
3650
* Issue a HTTP request and retry on failure.
@@ -41,8 +55,7 @@ public class FroniusHttpUtil {
4155
* @return the response body
4256
* @throws FroniusCommunicationException when the request execution failed or interrupted
4357
*/
44-
public static synchronized String executeUrl(HttpMethod httpMethod, String url, int timeout)
45-
throws FroniusCommunicationException {
58+
public String executeUrl(HttpMethod httpMethod, String url, int timeout) throws FroniusCommunicationException {
4659
return executeUrl(httpMethod, url, null, null, null, timeout);
4760
}
4861

@@ -59,17 +72,71 @@ public static synchronized String executeUrl(HttpMethod httpMethod, String url,
5972
* @return the response body
6073
* @throws FroniusCommunicationException when the request execution failed or interrupted
6174
*/
62-
public static synchronized String executeUrl(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
75+
public String executeUrl(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
76+
@Nullable InputStream content, @Nullable String contentType, int timeout)
77+
throws FroniusCommunicationException {
78+
return executeUrl(httpMethod, url, httpHeaders, content, contentType, timeout, this::executeRequest);
79+
}
80+
81+
/**
82+
* Issue a polling HTTP request and skip it when another request for the same bridge is already running.
83+
*
84+
* @param httpMethod the HTTP method to use
85+
* @param url the url to execute
86+
* @param timeout the socket timeout in milliseconds to wait for data
87+
* @return the response body
88+
* @throws FroniusCommunicationException when the request execution failed or interrupted
89+
*/
90+
public String executePollingUrl(HttpMethod httpMethod, String url, int timeout)
91+
throws FroniusCommunicationException {
92+
return executePollingUrl(httpMethod, url, null, null, null, timeout);
93+
}
94+
95+
/**
96+
* Issue a polling HTTP request and skip it when another request for the same bridge is already running.
97+
*
98+
* @param httpMethod the HTTP method to use
99+
* @param url the url to execute
100+
* @param httpHeaders optional http request headers which has to be sent within request
101+
* @param content the content to be sent to the given <code>url</code> or <code>null</code> if no content should be
102+
* sent.
103+
* @param contentType the content type of the given <code>content</code>
104+
* @param timeout the socket timeout in milliseconds to wait for data
105+
* @return the response body
106+
* @throws FroniusCommunicationException when the request execution failed or interrupted
107+
*/
108+
public String executePollingUrl(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
63109
@Nullable InputStream content, @Nullable String contentType, int timeout)
64110
throws FroniusCommunicationException {
111+
return executePollingUrl(httpMethod, url, httpHeaders, content, contentType, timeout, this::executeRequest);
112+
}
113+
114+
String executeUrl(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
115+
@Nullable InputStream content, @Nullable String contentType, int timeout, RequestExecutor requestExecutor)
116+
throws FroniusCommunicationException {
117+
return executeUrl(httpMethod, url, httpHeaders, content, contentType, timeout, RequestMode.CONTROL,
118+
requestExecutor);
119+
}
120+
121+
String executePollingUrl(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
122+
@Nullable InputStream content, @Nullable String contentType, int timeout, RequestExecutor requestExecutor)
123+
throws FroniusCommunicationException {
124+
return executeUrl(httpMethod, url, httpHeaders, content, contentType, timeout, RequestMode.POLLING,
125+
requestExecutor);
126+
}
127+
128+
private String executeUrl(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
129+
@Nullable InputStream content, @Nullable String contentType, int timeout, RequestMode requestMode,
130+
RequestExecutor requestExecutor) throws FroniusCommunicationException {
131+
acquireLock(requestLock, requestMode, url);
132+
LOGGER.debug("Executing {} request against {}", requestMode, url);
65133
int attemptCount = 1;
66134
try {
67135
while (true) {
68136
Throwable lastException = null;
69137
String result = null;
70138
try {
71-
result = HttpUtil.executeUrl(httpMethod.asString(), url, httpHeaders, content, contentType,
72-
timeout);
139+
result = requestExecutor.execute(httpMethod, url, httpHeaders, content, contentType, timeout);
73140
} catch (IOException e) {
74141
// HttpUtil::executeUrl wraps InterruptedException into IOException.
75142
// Unwrap and rethrow it so that we don't retry on InterruptedException
@@ -99,6 +166,30 @@ public static synchronized String executeUrl(HttpMethod httpMethod, String url,
99166
} catch (InterruptedException e) {
100167
Thread.currentThread().interrupt();
101168
throw new FroniusCommunicationException("Interrupted", e);
169+
} finally {
170+
requestLock.unlock();
102171
}
103172
}
173+
174+
private static void acquireLock(ReentrantLock requestLock, RequestMode requestMode, String url)
175+
throws FroniusCommunicationException {
176+
try {
177+
if (requestMode == RequestMode.POLLING) {
178+
if (!requestLock.tryLock()) {
179+
throw new FroniusPollingSkipException("Skipping polling request to '" + url
180+
+ "' because another request for this Fronius bridge is still running");
181+
}
182+
} else {
183+
requestLock.lockInterruptibly();
184+
}
185+
} catch (InterruptedException e) {
186+
Thread.currentThread().interrupt();
187+
throw new FroniusCommunicationException("Interrupted", e);
188+
}
189+
}
190+
191+
private @Nullable String executeRequest(HttpMethod httpMethod, String url, @Nullable Properties httpHeaders,
192+
@Nullable InputStream content, @Nullable String contentType, int timeout) throws IOException {
193+
return HttpUtil.executeUrl(httpMethod.asString(), url, httpHeaders, content, contentType, timeout);
194+
}
104195
}

‎bundles/org.openhab.binding.fronius/src/main/java/org/openhab/binding/fronius/internal/api/FroniusPollingSkipException.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Copyright (c) 2010-2026 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.binding.fronius.internal.api;
14+
15+
import org.eclipse.jdt.annotation.NonNullByDefault;
16+
17+
/**
18+
* Exception used when a polling request is skipped because another request for the same Fronius host is still active.
19+
*
20+
* @author Jimmy Tanagra - Initial contribution
21+
*/
22+
@NonNullByDefault
23+
public class FroniusPollingSkipException extends FroniusCommunicationException {
24+
private static final long serialVersionUID = 4310230808473861112L;
25+
26+
public FroniusPollingSkipException(String message) {
27+
super(message);
28+
}
29+
}

‎bundles/org.openhab.binding.fronius/src/main/java/org/openhab/binding/fronius/internal/handler/FroniusBaseThingHandler.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.openhab.binding.fronius.internal.FroniusBridgeConfiguration;
2323
import org.openhab.binding.fronius.internal.api.FroniusCommunicationException;
2424
import org.openhab.binding.fronius.internal.api.FroniusHttpUtil;
25+
import org.openhab.binding.fronius.internal.api.FroniusPollingSkipException;
2526
import org.openhab.binding.fronius.internal.api.dto.BaseFroniusResponse;
2627
import org.openhab.binding.fronius.internal.api.dto.Head;
2728
import org.openhab.binding.fronius.internal.api.dto.HeadStatus;
@@ -32,6 +33,7 @@
3233
import org.openhab.core.thing.ThingStatus;
3334
import org.openhab.core.thing.ThingStatusDetail;
3435
import org.openhab.core.thing.binding.BaseThingHandler;
36+
import org.openhab.core.thing.binding.ThingHandler;
3537
import org.openhab.core.types.Command;
3638
import org.openhab.core.types.RefreshType;
3739
import org.openhab.core.types.State;
@@ -151,6 +153,9 @@ public void refresh(FroniusBridgeConfiguration bridgeConfiguration) {
151153
if (getThing().getStatus() != ThingStatus.ONLINE) {
152154
updateStatus(ThingStatus.ONLINE);
153155
}
156+
} catch (FroniusPollingSkipException e) {
157+
logger.debug("Skipping refresh for {} because another request is already in progress.",
158+
getThing().getUID().getId());
154159
} catch (FroniusCommunicationException | RuntimeException e) {
155160
logger.debug("Exception caught in refresh() for {}", getThing().getUID().getId(), e);
156161
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
@@ -166,19 +171,52 @@ public void refresh(FroniusBridgeConfiguration bridgeConfiguration) {
166171
protected abstract void handleRefresh(FroniusBridgeConfiguration bridgeConfiguration)
167172
throws FroniusCommunicationException;
168173

174+
protected @Nullable FroniusBridgeHandler getFroniusBridgeHandler() {
175+
Bridge bridge = getBridge();
176+
if (bridge == null) {
177+
return null;
178+
}
179+
ThingHandler bridgeHandler = bridge.getHandler();
180+
return bridgeHandler instanceof FroniusBridgeHandler froniusBridgeHandler ? froniusBridgeHandler : null;
181+
}
182+
183+
protected FroniusHttpUtil getHttpUtil() throws FroniusCommunicationException {
184+
FroniusBridgeHandler bridgeHandler = getFroniusBridgeHandler();
185+
if (bridgeHandler == null) {
186+
throw new FroniusCommunicationException("Bridge handler is not available");
187+
}
188+
return bridgeHandler.getHttpUtil();
189+
}
190+
169191
/**
192+
* Collect data for periodic polling. Requests are skipped when another request for the same bridge is in progress.
170193
*
171194
* @param type response class type
172195
* @param url to request
173196
* @return the object representation of the json response
174197
*/
175198
protected <T extends BaseFroniusResponse> T collectDataFromUrl(Class<T> type, String url)
176199
throws FroniusCommunicationException {
200+
return collectDataFromUrl(type, url, true);
201+
}
202+
203+
/**
204+
* Collect data and choose whether the request may be skipped while another request for the same bridge is active.
205+
*
206+
* @param type response class type
207+
* @param url to request
208+
* @param usePollingRequest true when the request may be skipped, false when it must wait for the active request
209+
* @return the object representation of the json response
210+
*/
211+
protected <T extends BaseFroniusResponse> T collectDataFromUrl(Class<T> type, String url, boolean usePollingRequest)
212+
throws FroniusCommunicationException {
177213
try {
214+
FroniusHttpUtil httpUtil = getHttpUtil();
178215
int attempts = 1;
179216
while (true) {
180217
logger.trace("Fetching URL = {}", url);
181-
String response = FroniusHttpUtil.executeUrl(HttpMethod.GET, url, API_TIMEOUT);
218+
String response = usePollingRequest ? httpUtil.executePollingUrl(HttpMethod.GET, url, API_TIMEOUT)
219+
: httpUtil.executeUrl(HttpMethod.GET, url, API_TIMEOUT);
182220
logger.trace("aqiResponse = {}", response);
183221

184222
@Nullable

0 commit comments

Comments
 (0)