Skip to content

Comments

[shelly] Fix "duplicate id" in device log#20094

Closed
markus7017 wants to merge 4 commits intoopenhab:mainfrom
markus7017:shelly_fixduplid
Closed

[shelly] Fix "duplicate id" in device log#20094
markus7017 wants to merge 4 commits intoopenhab:mainfrom
markus7017:shelly_fixduplid

Conversation

@markus7017
Copy link
Contributor

@markus7017 markus7017 commented Jan 18, 2026

Users do report "duplicate id" in the device log. This relates to the fact that the device might receive overlapping http requests from thing and mDNS discovery threads. Each thread started with id 1, which is inconsistent (each id should be different).

The following changes were made

  • RPC message id starts with a random base for each thread and is then incremented for every message
    the request ID as a suffix makes it unique across threads.
  • The id is incremented of a request is resend due to auth handling (on http 401)
  • RPC src must also be unique (due to potentially overlapping http requests), therefore is build in the format "ohshelly--<requestId) where source is the local IP for discoveries or the thing uid when calling from a thing handler. Using - The config.serviceName and config.localIP were not consistently initialized, fixed

With those changes I don't see anymore "duplicate id" in the device log and RPC.src field is correctly build.

Awaiting #20069

Closing #19223

Signed-off-by: Markus Michels <markus7017@gmail.com>
requested id includes "ohshelly-<id>-<request id>"

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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes "duplicate id" warnings in Shelly device logs by ensuring unique RPC message IDs and source identifiers across concurrent HTTP requests from different threads (thing handlers and mDNS discovery).

Changes:

  • Modified RPC message ID generation to use thread-safe atomic counters with random starting values
  • Changed RPC source field format to include unique identifiers (thing UID or local IP) plus request ID
  • Updated API initialization to consistently pass configuration parameters
  • Fixed config initialization to properly set serviceName and localIp during discovery

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Shelly2ApiClient.java Implements thread-safe request ID generation using AtomicInteger with random base, builds unique RPC source identifiers
Shelly2ApiRpc.java Updates initialize method signature to accept config, regenerates request on auth retry
ShellyBaseHandler.java Calls api.initialize with config parameters, optimizes setConfig calls
ShellyBasicDiscoveryService.java Initializes config with serviceName and localIp, removes unused IOException, fixes empty hostname handling
ShellyApiInterface.java Updates initialize method signature to require thingName and config parameters
Shelly1HttpApi.java Implements new initialize signature
ShellyBluApi.java Delegates to parent initialize implementation
Shelly2ApiJsonDTO.java Adds jsonrpc field to RPC message structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1012 to +1015
request.src = RPC_SRC_PREFIX + suffix + "-" + request.id; // use a unique identifier; request.method =
// !method.contains(".") ?
// SHELLYRPC_METHOD_CLASS_SHELLY + "." + method :
// method;
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The request.method field is not being assigned. The code in the comment on lines 1012-1015 shows the intended logic but it's been commented out rather than properly implemented. The method parameter should be assigned to request.method, with the logic: request.method = !method.contains(".") ? SHELLYRPC_METHOD_CLASS_SHELLY + "." + method : method;. Without this assignment, all RPC requests will have a null method field, causing them to fail.

Suggested change
request.src = RPC_SRC_PREFIX + suffix + "-" + request.id; // use a unique identifier; request.method =
// !method.contains(".") ?
// SHELLYRPC_METHOD_CLASS_SHELLY + "." + method :
// method;
request.src = RPC_SRC_PREFIX + suffix + "-" + request.id; // use a unique identifier
request.method = !method.contains(".") ? SHELLYRPC_METHOD_CLASS_SHELLY + "." + method : method;

Copilot uses AI. Check for mistakes.
String uid = thing.getThing().getUID().getAsString();
suffix = substringAfterLast(uid, ":");
} else {
suffix = config.localIp; // use a unique identifier;
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

If config.localIp is empty when thing is null (during discovery), the suffix variable will be an empty string, resulting in request.src being formatted as "ohshelly--{requestId}" with a double dash. While this should still function, it would be clearer to handle this case explicitly, perhaps by using a default placeholder like "discovery" or "unknown" when localIp is empty.

Suggested change
suffix = config.localIp; // use a unique identifier;
String localIp = config.localIp;
if (localIp == null || localIp.isEmpty()) {
suffix = "discovery"; // fallback identifier when localIp is not available
} else {
suffix = localIp; // use a unique identifier;
}

Copilot uses AI. Check for mistakes.
@openhab-bot
Copy link
Collaborator

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

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

@markus7017
Copy link
Contributor Author

Replace by PR #20248

@markus7017 markus7017 closed this Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting other PR Depends on another PR bug An unexpected problem or unintended behavior of an add-on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants