Re: API Changes proposal

[Christofer Dutz]

Hi all,

just wanted to give you an update on my progress.

I started with updating the examples and integrations and quickly came to a 
point, where I had to continue finishing the API refactoring and the 
base-driver refactoring.

So I just committed my last changes that should make it possible to build Read 
& Write-Requests. I really hope to finish this refactoring in the next two days 
as it's totally driving me nuts.
For the last few days every dream at night has been dealing with architectural 
problems, byte encoding and stuff like that ... that has to change ;-)

Just as an example ... the new S7PlcFieldHandlerTest now runs additional 7178 
individual tests to test all combinations of Java and S7 type combinations and 
their respective value ranges (MIN, MAX, 0, Some random value).
I still need to implement the temporal types Time, Date and DateTime, and test 
the "ULWORD" types, but I guess most should be somewhat usable. 
Wouldn't have thought that the Write direction was so much more work than the 
Read path.

So much for the Update ...

Chris



Am 03.09.18, 13:53 schrieb "Julian Feinauer" :

Hi Chris,

exactly, that was my point (sorry for writing it not out clearly).
We can do it that way the only thing we are "loosing" is to know whether 
bit was given by the user explicitly or not.
I dont know if we need this after parsing is finished anymore.

So we can also do it your way.

Julian

Am 03.09.18, 10:47 schrieb "Christofer Dutz" :

Hi Julian,

had to think a little to get your point ... but think I have ... In my 
example it's a primitive "short" and in yours it's "Short" as nullable 
non-primitive-type.
I don't technically prefer any of the two options. Physically the 
bit-offset of any non-bit type is always 0 and not null (as in undefined) as 
every non-bit value always starts at bit 0 ... so for that reason I would 
prefer "short" with default 0 over the "Short" nullable version. And this way 
we don't have to add null-checks in the code. But as I said ... that's a very 
slight preference for that option.

Chris

Am 03.09.18, 10:27 schrieb "Julian Feinauer" 
:

Hi chris,

I agree with your S7 field except for one little change.
How do we proceed with the (optional) bit offset?
I made it "Short" with the contract that null indicates no offset 
given.
Another alternative would be to make it 0 as default or even 
Optional.
I would prefer to have it nullable, what do you think?

With the rest I'm fine but as this is part of our internal API now 
I think we also have more freedom with evolving them as its not visible to 
users.

For all other parts of your proposal +1 from me.

Best
Julian

Am 30.08.18, 10:15 schrieb "Christofer Dutz" 
:

Hi all,

especially @Julian ... could you please have a look at that I 
did with the S7Field [1]?
Also there is a unit-test that should allow adding more 
statements and testing everything is working ok [2].

Does this match your idea on [3]? Looking at your addresses, I 
think that I might have not quite got it ... is there always a "D" as first 
part after the "."? I always read it as "DB" like Data Block ... but seeing DX 
and SW makes me wonder ... a quick check in my TIA shows me the address of a 
Boolean field in a Data Block is "%DB1.DBX38.1" ... which one is correct?

As we're no longer constructing the objects themselves in the 
API, I took the liberty to simplify the field objects so we now only have one 
type for S7.

Chris

[1] 
https://github.com/apache/incubator-plc4x/blob/feature/api-redesign-chris-c/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java
[2] 
https://github.com/apache/incubator-plc4x/blob/feature/api-redesign-chris-c/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/s7/model/S7FieldTests.java
[3] 
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89070222


Am 28.08.18, 12:23 schrieb "Christofer Dutz" 
:

Hi all,

I just pushed changes to my API refactoring branch ... so 
far I only adjusted the API module and added an example using the changed API.
To have a look, please go to [1] ...

General changes I implemented while working on the 
refactoring itself. I did notice, that my current proposal "chris-2" did 

Having to 

[GitHub] chrisdutz commented on a change in pull request #14: WIP: Api Refactoring

[GitBox]

chrisdutz commented on a change in pull request #14: WIP: Api Refactoring
URL: https://github.com/apache/incubator-plc4x/pull/14#discussion_r215170934
 
 

 ##
 File path: 
plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcFieldRequest.java
 ##
 @@ -20,11 +20,13 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 import org.apache.plc4x.java.api.model.PlcField;
 
-import java.util.Collection;
+import java.util.LinkedHashSet;
 
 public interface PlcFieldRequest extends PlcRequest {
 
-Collection getFieldNames();
+int getNumFields();
+
+LinkedHashSet getFieldNames();
 
 Review comment:
   Because that is the most generic container that is: 
   - A Map (We need to map names to obects)
   - Preserves the order (might be obsolete now ... have to re-check)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sruehl commented on a change in pull request #14: WIP: Api Refactoring

[GitBox]

sruehl commented on a change in pull request #14: WIP: Api Refactoring
URL: https://github.com/apache/incubator-plc4x/pull/14#discussion_r214037249
 
 

 ##
 File path: 
plc4j/api/src/main/java/org/apache/plc4x/java/api/messages/PlcWriteRequest.java
 ##
 @@ -24,6 +24,8 @@ Licensed to the Apache Software Foundation (ASF) under one
 
 public interface PlcWriteRequest extends PlcFieldRequest {
 
+int getNumValues(String name);
 
 Review comment:
   in the java world one would name these `getNumberOfValues` even if this is a 
bit more verbose but these shortcuts are not seen that much. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services