Hello Chris,
I believe both ways are fine, but if speak of simplicity the shorter
version wins. Given that after discussed changes unsubscribe call will
not need a builder, we can construct it for user request in one pass.
One consideration I'd make is keeping reference to smaller object rather
than larger ones. This means that
subscriptionResponse.getUnsubscriptionRequest() might be a better fit
than retaining entire subscription response which has much more fields
(field statuses etc.).
Best,
Łukasz
On 26.03.2024 10:17, Christofer Dutz wrote:
Hi all,
So, I’ve been fefactoring the Subscription stuff in a branch (not yet pushed
however).
I stumbled over something:
In the past we had an unsubscription request that took “subscription handles”
This made sense as we had one subscription handle per field.
Now that we’re changing things to have a callback for all fileds in the
subscription request, I think this method no longer really makes much sense.
I would now rather have someone keep the subscription request and call an
“unsubscribe” method on this.
So as soon as you’re finished with your subscription a
“subscriptionResponse.unsubscribe()” call would take care of that.
Or possibly a “subscriptionResponse.getUnsubscriptionRequest().execute()”
(which might be more in line with our usual patterns.
This way also someone could drop the subscription response and save the
unsubscriptionRequest till he needs it.
What do you folks think?
Chris
Von: Christofer Dutz <christofer.d...@c-ware.de>
Datum: Montag, 25. März 2024 um 09:32
An: dev@plc4x.apache.org <dev@plc4x.apache.org>
Betreff: AW: [DISCUSS] Where to regster the handler?
Having another look at existing types:
In browse-request we have:
* execute()
* executeWithInterceptor()
In discovery-request we have:
* execute()
* executeWithHandler()
So, in order to continue that pattern, what do you think about having two
options:
* execute()
* executeWithHandler()
And to add the option to add a handler to the response.
I know this could let users miss first responses, especially as ADS for example
sends an event directly after subscribing.
However it would be more in line with the rest of the API, which all have an
“execute” without any arguments.
Thinking even more about everything … I think moving the
“executeWithInterceptor” .. the interceptor adding should actually more be
added to the request and not the execution.
Also could we add a “WithHandler” option to every type of request.
Chris
Von: Christofer Dutz <christofer.d...@c-ware.de>
Datum: Montag, 25. März 2024 um 09:15
An: dev@plc4x.apache.org <dev@plc4x.apache.org>
Betreff: [DISCUSS] Where to regster the handler?
Hi all,
so yesterday I‘ve been working on the refactoring of the subscriptions as we
discussed that on the meetup.
So, we said to register a callback for all fields before the “execute()”
Consumer<PlcSubscriptionEvent> subscriptionConsumer = …;
PlcSubscriptionRequest.Builder builder =
plcConnection.subscriptionRequestBuilder();
for (int i = 0; i < options.getTagAddress().length; i++) {
builder.addChangeOfStateTagAddress("value-" + i,
options.getTagAddress()[i]);
}
PlcSubscriptionRequest subscriptionRequest =
builder.build(subscriptionConsumer);
// Execute the subscription response.
final PlcSubscriptionResponse subscriptionResponse =
subscriptionRequest.execute().get();
Right now, I initially thought about adding it to the build()-method, but that
sort of felt oddly.
Thinking about it a bit more, adding it to the “execute” seemed a bit more like
what I was looking for.
So, what do you think about doing:
Consumer<PlcSubscriptionEvent> subscriptionConsumer = …;
PlcSubscriptionRequest.Builder builder =
plcConnection.subscriptionRequestBuilder();
for (int i = 0; i < options.getTagAddress().length; i++) {
builder.addChangeOfStateTagAddress("value-" + i,
options.getTagAddress()[i]);
}
PlcSubscriptionRequest subscriptionRequest = builder.build();
// Execute the subscription response.
final PlcSubscriptionResponse subscriptionResponse =
subscriptionRequest.execute(subscriptionConsumer).get();
What’s your opinion? I prefer the adding to the “execute” method.
Chris
