You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Christofer Dutz <ch...@c-ware.de> on 2024/03/25 08:14:18 UTC
[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
AW: AW: [DISCUSS] Where to regster the handler?
Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Lukasz,
I fully agree … that’s why I already started implementing that path a bit in order to get a feeling on how it would be used.
Admittedly I’m in strong favor of calling “getUnsubscriptionRequest()” on the SubscriptionResponse object.
Chris
Von: Łukasz Dywicki <lu...@code-house.org>
Datum: Dienstag, 26. März 2024 um 10:30
An: dev@plc4x.apache.org <de...@plc4x.apache.org>
Betreff: Re: AW: [DISCUSS] Where to regster the handler?
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 <ch...@c-ware.de>
> Datum: Montag, 25. März 2024 um 09:32
> An: dev@plc4x.apache.org <de...@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 <ch...@c-ware.de>
> Datum: Montag, 25. März 2024 um 09:15
> An: dev@plc4x.apache.org <de...@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
>
Re: AW: [DISCUSS] Where to regster the handler?
Posted by Łukasz Dywicki <lu...@code-house.org>.
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 <ch...@c-ware.de>
> Datum: Montag, 25. März 2024 um 09:32
> An: dev@plc4x.apache.org <de...@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 <ch...@c-ware.de>
> Datum: Montag, 25. März 2024 um 09:15
> An: dev@plc4x.apache.org <de...@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
>
AW: [DISCUSS] Where to regster the handler?
Posted by Christofer Dutz <ch...@c-ware.de>.
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 <ch...@c-ware.de>
Datum: Montag, 25. März 2024 um 09:32
An: dev@plc4x.apache.org <de...@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 <ch...@c-ware.de>
Datum: Montag, 25. März 2024 um 09:15
An: dev@plc4x.apache.org <de...@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
AW: [DISCUSS] Where to regster the handler?
Posted by Christofer Dutz <ch...@c-ware.de>.
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 <ch...@c-ware.de>
Datum: Montag, 25. März 2024 um 09:15
An: dev@plc4x.apache.org <de...@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