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 <l...@code-house.org>
Datum: Dienstag, 26. März 2024 um 10:30
An: dev@plc4x.apache.org <dev@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 <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
>
