Hi Chris, [1] no need to supply a text to „ PlcInvalidFieldException“ as it only requires you to supply the field name. [2] using junit5 for parameterd tests seems to be a big relief :)
You should add a Pull-Request with WIP: (work in progress fix) so we can add remarks like above inline as they are not really worth a mail on the mailing list. What do you think. Sebastian > Am 30.08.2018 um 10:14 schrieb Christofer Dutz <christofer.d...@c-ware.de>: > > 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" <christofer.d...@c-ware.de>: > > 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 inject the type conversion code would have made it necessary to > inject a converter which didn't feel right. So I changed the API to be purely > interface based. > In order to be able to construct these objects I also added builders for > them. > > I asked a few people here what they think, and most liked the simplicity > and didn't have any WTF experiences (Which seems to be a good thing as I did > have to explain a lot with the current API) > > Quick Feedback highly appreciated as I will start implementing > DefaultPlcReadRequest & Co (in driver-base ... together with the builders) > after that I'll start migrating the drivers. > Right now having a look a named example [1] would be a good start ... > Second would be a deeper look into the API module [2]. > > Would be a shame to waste that time and effort if you think the changes > suck (or are less than optimal as non-Germans would probably call them ;-) ) . > > Chris > > [1] > https://github.com/apache/incubator-plc4x/blob/feature/api-redesign-chris-c/examples/hello-plc4x/src/main/java/org/apache/plc4x/java/examples/helloplc4x/HelloPlc4x.java > [2] > https://github.com/apache/incubator-plc4x/tree/feature/api-redesign-chris-c/plc4j/api > > > Am 27.08.18, 09:57 schrieb "Christofer Dutz" <christofer.d...@c-ware.de>: > > Ups ... after reloading .. I just saw Julians Proposal pop up ... > haven't looked into that ... > Will do that right away. > > Chris > > Am 25.08.18, 15:52 schrieb "Christofer Dutz" > <christofer.d...@c-ware.de>: > > Hi Julian, > > version 2 should now be quite different ... I started reworking my > original proposal and decided to revert that an start a second proposal. > My first did address some parts needing cleaning up, but I still > wasn't quite satisfied with it. So I did another more radical refactoring. > > If you reload the second there should be a lot of differences. > > I just hit "save" a few minutes ago however ... but now I'm quite > happy with it. So please have another look at the second proposal. > > And please, maybe add your own proposal ... my versions are just > Brainstorming from my side. > > My favorite is currently "Chris' Proposal 2" ;-) > > Chris > > > > >
