Hey,
a side note to chris note on the example code.
Should we have an explicit example repo which we always keep in sync and which
we can reference in such articles with a note that things can be slightly
different?
Julian
Am 08.10.18, 17:34 schrieb "Christofer Dutz" <christofer.d...@c-ware.de>:
Also it would be super cool, if we could merge this soon as I'm currently
writing an Article on PLC4X and would be great if the example code wasn't
obsolete at the time I'm submitting the text (Of course I'm expecting it to be
slightly off till we print it) ;-)
Chris
Am 08.10.18, 11:41 schrieb "Andrey Skorikov"
<andrey.skori...@codecentric.de>:
Hi Chris,
this makes sense to me :-) If we do go down this path, we should
consider that some information is:
- driver specific: what capabilities does this particular protocol
implementation support
- protocol specific: what capabilities (for example
writing/subscription) does the protocol provide in general
- connection specific: for example, whether the connection is
encrypted, authentication/authorization used etc.
- device specific: what capabilities does the connected device
provide
(might be a subset of protocol capabilities)
We should be careful when designing that metadata interface and not mix
these things up, to avoid confusion. For example, it should be clear to
the client that in case subscription is not supported, whether this is
a
driver (protocol implementation) issue, a protocol issue, or device
issue.
Andrey
On 10/08/2018 11:01 AM, Christofer Dutz wrote:
> Hi Andrey,
>
> Ah ok ... now I understand. I agree that I also like this approach
... it keeps the connection cleaner.
> And I guess such a Metadata object could not only contain such
information about the capabilities, but also the concrete type of the PLC a
connection is connected to, Versions etc.
> I could imagine that some supported functions are not only limited by
the driver itself, but by the PLC model used. At least the supported datatypes
is highly dependent on the type of S7 device.
> So I would definitely +1 to go down this Metadata path.
>
> Chris
>
>
>
> Am 07.10.18, 19:46 schrieb "Andrey Skorikov"
<andrey.skori...@codecentric.de>:
>
> Hi Chris,
>
> I agree. As for now, the PR is already quite large and I would
not like
> to let it grow further.
>
> A metadata object returned by some operation on PlcConnection
(for
> example getMetadata() or getCapabilities()) would bundle all the
> operations for obtaining information about the connection
itself. This
> is in contrast to the operational interface of the connection,
which is
> used to actually perform the operations like reading/writing.
Basically,
> all the canXYZ operations discussed so far can be bundled into
one
> interface, and that would constitute the management interface
(for
> obtaining metainformation) of the collection.
>
> As Julian pointed out, this pattern is employed in the java.sql
API:
>
https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html.
> The corresponding operation to obtain an instance of that type is
>
https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html#getMetaData().
>
> Another example is the JMX instrumentation level API for dynamic
beans:
>
https://docs.oracle.com/javase/7/docs/api/javax/management/DynamicMBean.html#getMBeanInfo().
>
> However, I believe that at this stage there is no need to
provide a
> separate interface for that, and having simple
canRead()/canWrite()
> directly on PlcConnection would be sufficient.
>
> Andrey
>
>
> On 10/07/2018 06:17 PM, Christofer Dutz wrote:
> > Hi Julian,
> >
> > I agree that we should merge things asap ... just because
something is merged, doesn't mean we can't fine-tune it after that.
> > I did have a look at the changes and I think it's safe to
continue down that path.
> >
> > Also I like the idea of getting rid of the Optional ... it was
annoying me too for quite some time. So having a "canXYZ" and a companion
"getXYZRequestBuilder" methods sounds perfect from my side.
> > This way we can go the extra step of ensuring support, but can
omit it where we just don't need it.
> >
> > Haven't quite understood the whole "Metadata" thing though ...
;-)
> >
> > Chris
> >
> >
> > Am 07.10.18, 15:15 schrieb "Julian Feinauer"
<j.feina...@pragmaticminds.de>:
> >
> > Hey all,
> >
> > one more question.
> > Do we do the suggested changes in Andreys PR Branch or do
we do it separately.
> > Then, we should try to merge this branch ASAP to have it
there and to avoid merge hell (see
https://media.giphy.com/media/cFkiFMDg3iFoI/giphy.gif).
> >
> > Personally, I feel unable to do a Code Review in the
original sense (105 changes).
> > So after going through the API changes I definitely +1
them but I'm unsure if a "proper" Code Review is possible / necessary (so would
agree on merging directly).
> >
> > What do others think?
> >
> > Julian
> >
> > Am 06.10.18, 21:20 schrieb "Julian Feinauer"
<j.feina...@pragmaticminds.de>:
> >
> > Hey Andrey,
> >
> > I have to admit that your naming is definetly better
than mine.
> > And I like your idea about this Metadata thing a lot.
> > I just checked how this is named in JDBC and the
respective class is
https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html
> >
> > So I think we can provide a canRead / canWrite
(canSubscribe is a bit difficult, as we already hat several discussions about
if we implement that by polling by default).
> > But I would also like the idea of having such a
Metadata interface to transport further information about the PLC (like if this
is native subscribing or polling and all such stuff).
> >
> > Best
> > Julian
> >
> > Am 06.10.18, 21:08 schrieb "Andrey Skorikov"
<andrey.skori...@codecentric.de>:
> >
> > Hello Julian,
> >
> > I think that a
canRead()/canWrite()/canSubscribe() methods signaling
> > whether the connection supports
reading/writing/subscription is a really
> > good solution. This would cleanly separate
querying the meta-information
> > of a connection (whether the connection provides
the required
> > capability) from actually using it, and would
free the client from
> > dealing with the Optional<?>s all the time.
> >
> > There are also some alternative solutions:
> >
> > - Provide the meta-information in a separate data
structure, returned by
> > some operation like getCapabilites() on
PlcConnection. This can be
> > modeled in great detail or very simply (for
example by returning a
> > BitSet). The client would check whether the
required operation is
> > supported by calling operations on that object.
> >
> > - Provide "mix-in" interfaces, for example
Readable and Writable. The
> > client would check whether the connection
supports reading by evaluating
> > whether the connection object implements the
required interface (for
> > example: connection instanceof Readable) and
casting the connection to
> > that type.
> >
> > - Provide no meta-information at all and just
throw an exception when a
> > unsupported operation is invoked. Would not
recommend that, but still :-)
> >
> > In total, I think that Julian's solution
(canRead() with Exception
> > thrown when a unsupported operation is invoked)
balances the complexity
> > and flexibility best.
> >
> > Andrey
> >
> >
> > On 10/06/2018 08:38 PM, Julian Feinauer wrote:
> > > Hey everybody,
> > >
> > > I’m currently groing through Andreys PR
(https://github.com/apache/incubator-plc4x/pull/27) which introduces some very
good API changes and makes the API a lot more concise.
> > > But one thing that annoys me from the first day
on plc4x is still there (and is now even more annoying as the rest is so
clean). It is the boilerplate code I have write all the time when “just doing a
connection to read something” due to the Optional<?> for getting the reader (or
now the ReadRequestBuilder).
> > > For me, the getReader (or now
readRequestBuilder) as Optional is like what Sebastian hates about Checked
Exceptions.
> > > I never had to deal with a Connection which did
not have a Reader but I had to check the Optional… at least 50 times, perhaps
even more.
> > >
> > > Can’t we come up with a solution for that which
would make the API (from my perspective) even more clean and user friendly.
> > >
> > > Suggestions could be:
> > >
> > > 1. Replace the connection directly with
Reader, so no getConnection but getReader (or readRequestBuilder). And if this
fails, it throws a PlcConnectionException, as usual.
> > > 2. No optional but another or canRead()
method (for those who like it save) and it then throws a unchecked
PlcConnectionException (or some subclass)
> > >
> > > What do the others think? Is this only me
having the feeling that this is the same anti-pattern as with the checked
exceptions?
> > >
> > > Julian
> >
> >
> >
> >
> >
> >
> >
>
>
>
