Hi Chris,
no need for a factory-factory. :) I believe that the core problem is
that the PlcConnection interface does too much - it offers
protocol-specific transport functionality by providing the
PlcReader.read operation, it maintains protocol state, and it serves as
a factory for read requests (through PlcReader.readRequestBuilder).
Another problem is that the requests, which are obtained through the
ReadRequestBuilder are basically independent from a concrete connection
instance. Hence it does not make sense to obtain a Builder instance from
a PlcConnection. For example, consider the following scenario: first we
obtain a PlcConnection, PlcReader and a RequestBuilder off it. Then we
close the PlcConnection. What should happen, if we build and try to
execute requests from that RequestBuilder? Should it return request
instances but throw exceptions when we try to execute them on another
(live) connection? Or should it throw exceptions when we call build()?
Or should it allow us to execute the built requests on another
connection? In the latter case, why should be forced to obtain a request
builder from a connection instance anyway?
Best regards,
Andrey
On 09/12/2018 04:21 PM, Christofer Dutz wrote:
Hi Andrey,
are you proposing a Factory-Factory? Wouldn't that go a little too far?
Currently the request factory method being in the connection, is just an
implementation detail we could have a S7Reader class implementing the Reader
interface and this is generated from the connection, if this is the problem.
Chris
Am 12.09.18, 15:54 schrieb "Andrey Skorikov" <[email protected]>:
Hi Sebastian,
good point! The mutability of PlcReadRequest would be a consequence of
the mutability of the PlcConnection (or something that can handle the
execute). However, in order to construct a immutable PlcReadRequest,
currently we still need to obtain a PlcRequest.Builder from the same
mutable PlcConnection. I think, if we really want PlcReadRequest to be
immutable, then we should be able to construct them independently (not
from a mutalbe object). Perhaps we could separate PlcRequest
construction from its exection by providing a RequestBuilder factory
method not on a mutable PlcConnection but "higher up", for example
somewhere on a PlcDriver?
Regards,
Andrey
On 09/12/2018 03:33 PM, Sebastian Rühl wrote:
> Hey Andrey,
>
> One thing that would stand against this: Adding a execute() would make
the PlcReadRequest mutable which is a thing we should avoid (Mutable because it
would need a reference store to something that can handle the execute).
>
> FYI: I added a mutability test into plc4j-api (which should be added to
plc4j-driver-bases after the refactoring) which tests all Items for mutability.
Currently we have some open issues regarding that.
>
> Sebastian
>
>> Am 12.09.2018 um 14:50 schrieb Andrey Skorikov
<[email protected]>:
>>
>> Hello,
>>
>> currently, PlcReadRequests and PlcWriteRequests are executed by
invoking the corresponding read/write operation on the PlcReader/PlcWriter objects.
Hence the typical workflow for reading a value contains the following steps:
>>
>> 1. Obtain a PlcReader instance from a PlcConnection
>> 2. Obtain a Builder by invoking readRequestBuilder() on PlcReader
>> 3. Build a request using the Builder
>> 4. Execute the request by invoking read() on the PlcReader, passing the
constructed request as argument
>>
>> PlcReader reader = ... // obtain reader
>> PlcReadRequest request =
reader.readRequestBuilder().addItem(...).build();
>> Future<PlcReadResponse> response = reader.read(request);
>>
>> Since we only can build a request throgh a PlcReader instance, invoking
the read operation on the PlcReader is redundant. Therefore I suggest removing the
read/write operation from the PlcReader/PlcWriter and defining a execute() operation
on PlcRequest instead. The workflow would look like this then:
>>
>> PlcReader reader = ... // obtain reader
>> PlcReadRequest request =
reader.readRequestBuilder().addItem(...).build();
>> Future<PlcReadResponse> response = request.execute();
>>
>> Please note that there is no more need to "keep" a reference to
PlcReader in this context, since it is implicitly associated with the request by calling
reader.readRequestBuilder().build().
>>
>> Best regards,
>> Andrey
>>
