Re: API Changes proposal
Perfect, then we do it that way. I feel a bit sorry for you that you did most of the heavy lifting and I'm like standing next to you giving bad comments like "nah, it would be better doing it that way". But when we meet in Nürtingen I owe you a beer for that __ Julian Am 06.09.18, 11:29 schrieb "Christofer Dutz" : Hi Julian, I think that would be ideal ... as this way I don't feel like moving things underneath your feet all the time ;-) After my change marathon yesterday I am hopeful that I will be able to finish this this week. Chris Am 06.09.18, 10:53 schrieb "Julian Feinauer" : Hi Chris, thank you so much for your effort! I can't wait for the refactoring to be finished (and the release of course). I'm following your branch and as you implemented most of the things we discussed I think its best to wait till you are finished and merge and then start off with the new S7 Syntax based on your branch. Best Julian Am 05.09.18, 22:55 schrieb "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
Re: API Changes proposal
Hi Julian, I think that would be ideal ... as this way I don't feel like moving things underneath your feet all the time ;-) After my change marathon yesterday I am hopeful that I will be able to finish this this week. Chris Am 06.09.18, 10:53 schrieb "Julian Feinauer" : Hi Chris, thank you so much for your effort! I can't wait for the refactoring to be finished (and the release of course). I'm following your branch and as you implemented most of the things we discussed I think its best to wait till you are finished and merge and then start off with the new S7 Syntax based on your branch. Best Julian Am 05.09.18, 22:55 schrieb "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
Re: API Changes proposal
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
Re: API Changes proposal
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 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" : 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" : 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
Re: API Changes proposal
Hi Sebastian, Yeah ... I thought that we last removed JUnit5 again as it was completely buggy, but having now read some "Yeah till the latest releases it really sucked, but now it works fine" I thought, why not allow it and we'll see how it works. I wouldn't propose to migrate everything however. And yes ... the parametrized tests were what made me try it out ;-) Well the PR is a great idea ... let's see if I can add a PR inside our own repo ... never done that. But it will not prevent emails about it ;-) I'll try that right away ... By the way ... due to the changes, the Plc4XS7Protocol class seems to become a lot simpler (And the lower levels shouldn't be affected). Chris Am 30.08.18, 10:53 schrieb "Sebastian Rühl" : 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 : > > 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 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" : > >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" : > >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
Re: API Changes proposal
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 : > > 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 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" : > >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" > : > >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 > > > > >
Re: API Changes proposal
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 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" : 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" : 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
Re: API Changes proposal
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" : 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" : 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
Re: API Changes proposal
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" : 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 Am 25.08.18, 15:32 schrieb "Julian Feinauer" : Hi Chris, I’m about to go through your suggestions fort he API changes (by the way, thanks for your effort and the very nice documentation) and I’m a bit confused about the differences between * Chris Proposal * Chris Proposal 2 They seem very equal to me. Are these two versions of the same proposal or two different proposals? Thanks! Julian
Re: API Changes proposal
Hi all,
I would strongly like to encourage you to have a look as the API Changes we
have listed in Confluence.
With the option to have diagrams, I think discussing such changes is a lot
easier than in pure text form here.
Currently I am in strong favor of my second proposal [1]
Major changes in this:
- Address is renamed to PlcField and now can contain type information.
- We now have every field aliased with a name, which is used to access this
field.
- Complete elimination of the {Read/Write}{Request/Response}Items. The data and
state is now included in the {Read/Write}{Request/Response} itself.
- The ReadResponse for example gets additional getIngeger(), getFloat(),
getString() methods, which come in two options:
- Just the field alias, which returns the first item value (single
value fields)
- Both the field alias as well as an index to access any item value
(multi value fields)
- The ReadResponse contains it's data in form of a byte-array containing the
raw data returned from the PLC.
- The conversion from PLC to Java types is done via PlcTypeConverters which are
part of the driver implementation and handle the mapping (Decoding is done when
accessing it, not when parsing the response).
- Code accessing the values now looks very similar to the code used in JDBC
applications (However not supporting the index-access ... but we could also
allow that)
Please check this out ... as I would like to start implementing any changes we
decide on ASAP ... after all I have 2 weeks before the start of the next
Publicity Marathon (4 Conferences and 2 Articles within the next 6 Weeks) ... I
would really like to have my slides and articles in a somewhat stable form and
not publish things that are changed a few weeks later.
Chris
[1] https://cwiki.apache.org/confluence/display/PLC4X/Chris%27+Proposal+2
Am 25.08.18, 15:52 schrieb "Christofer Dutz" :
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
Am 25.08.18, 15:32 schrieb "Julian Feinauer" :
Hi Chris,
I’m about to go through your suggestions fort he API changes (by the
way, thanks for your effort and the very nice documentation) and I’m a bit
confused about the differences between
* Chris Proposal
* Chris Proposal 2
They seem very equal to me.
Are these two versions of the same proposal or two different proposals?
Thanks!
Julian
Re: API Changes proposal
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 Am 25.08.18, 15:32 schrieb "Julian Feinauer" : Hi Chris, I’m about to go through your suggestions fort he API changes (by the way, thanks for your effort and the very nice documentation) and I’m a bit confused about the differences between * Chris Proposal * Chris Proposal 2 They seem very equal to me. Are these two versions of the same proposal or two different proposals? Thanks! Julian
API Changes proposal
Hi Chris, I’m about to go through your suggestions fort he API changes (by the way, thanks for your effort and the very nice documentation) and I’m a bit confused about the differences between * Chris Proposal * Chris Proposal 2 They seem very equal to me. Are these two versions of the same proposal or two different proposals? Thanks! Julian
