plc4x-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christofer Dutz <christofer.d...@c-ware.de>
Subject Re: API Changes proposal
Date Thu, 06 Sep 2018 09:28:50 GMT
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" <j.feinauer@pragmaticminds.de>:

    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" <christofer.dutz@c-ware.de>:
    
        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" <j.feinauer@pragmaticminds.de>:
        
            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" <christofer.dutz@c-ware.de>:
            
                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" <j.feinauer@pragmaticminds.de>:
                
                    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" <christofer.dutz@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.dutz@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.dutz@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.dutz@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
                                    
                                      
                            
                            
                        
                        
                    
                    
                
                
            
            
        
        
    
    

Mime
View raw message