Skip to content

Conversation

@scamille
Copy link
Member

@scamille scamille commented Aug 16, 2020

Add new class Types.Dtl by taking the DateTime code and adjusting things.

Includes Unit Test (which is just the DateTime one adjusted for DTL as well)

I had an existing DTL converter to work from, but https://support.industry.siemens.com/cs/mdm/109773506?c=93833257483&lc=en-WW describes the data type pretty well.

Serge Camille added 3 commits August 16, 2020 22:31
Add new class Types.Dtl by taking the DateTime type and adjusting things.

Also add unit test with binary data calculated by hand. (Need to verify with actual S7 data)
@scamille scamille changed the title Fb dtl Add DTL type Aug 16, 2020
@mycroes
Copy link
Member

mycroes commented Aug 17, 2020

@scamille Although there's no code in #267, I assume this implements what's described there? Will try to review this ASAP.

@scamille
Copy link
Member Author

No this is not in any way associated with #267 , nor did I get my implementation details from there.

If you want me to make any changes, eg. to naming things, just let me know.

Some of the implementation details (eg. using MemoryStream in ToByteArray) where done so I could reuse the existing Types.Word and Types.DWord implementions to do the conversion for year and nanoseconds.

@mycroes
Copy link
Member

mycroes commented Aug 17, 2020

Hi @scamille I know there's no association (and there's no code in #267 either, so nothing you could've used in that sense), but if this PR conveys the same intention as #267 then I can close that one as well when this is merged 😉

@scamille
Copy link
Member Author

Yes of course. Since there is no code in the other PR, I can just assume that based on its title it attempted to implement the same thing as I did :-)

About Naming: Do you prefer Dtl / DTL, or DateTimeLong? (Not to be confused with LDT (DATE_AND_LTIME) )

@mycroes
Copy link
Member

mycroes commented Aug 17, 2020

Honestly I'd prefer DateTimeLong, because it's most precise and with a proper IDE (or R# 😉) typing dtl will complete to DateTimeLong just as well. OTOH I'm not sure about the current conventions, they're not mine (I only did 'fixes' and write-multiple support, almost all of the other work is before I maintained S7NetPlus).

Copy link
Member

@mycroes mycroes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent implementation, very happy to see a contribution of good quality code.

@mycroes mycroes merged commit 3555436 into S7NetPlus:develop Aug 17, 2020
@mycroes
Copy link
Member

mycroes commented Aug 17, 2020

Released as 0.6.0.

@scamille scamille deleted the fb-DTL branch August 18, 2020 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants