Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address string with tokenisation, refactoring, loopback s7 c example added. s7c write variable works #233

Merged
merged 14 commits into from
May 3, 2021

Conversation

thomas169
Copy link
Member

  1. Changed address string parsing as in last cancel PR null checking of token is performed via strlen poxy. Removed timer/sleep features from windows versions
  2. Refactoring: reduced variable name verbosity, used more intermediate variables, and typically forwarded variable declarations to beginning of scopes, plus major refactoring on files relevant to next item.
  3. Added a write->read loopback test for s7 using bool/int8/int16 types
    3a) Added I think most todos in write.c, drive_s7_packet.c, driver_s7_sm_write.c

Note write response return values are not checked (or used), nor tested with arrays of a write type, (think it will not work)

@sruehl
Copy link
Contributor

sruehl commented Apr 10, 2021

@thomas169 is this still a WIP/Draft pull request? If yes I can mark it as such

@thomas169
Copy link
Member Author

Yeh still WIP. Write arrays (e.g. UINT[2]) work, but not sure the way I implemented this is how it was intended to be. As write response items are still unchecked / unused it's still draft.

@sruehl sruehl marked this pull request as draft April 12, 2021 06:57
… to write responces in sm_write.c, correct instalisation of request values in packets.c, naming consistency in sm_read.c.
@chrisdutz
Copy link
Contributor

Hi Thomas,

a general question ... could you be so kind and sign up for the dev@plc4x.apache.org list and tell us a bit about what you're up to?

Chris

@thomas169
Copy link
Member Author

I already signed up, but sure, will message mailing list.

@thomas169 thomas169 marked this pull request as ready for review April 13, 2021 14:40
@sruehl
Copy link
Contributor

sruehl commented Apr 26, 2021

out of curiosity: as you (@thomas169) marked it for ready for review and added changes to it 7 days after: is it done done yet? If so maybe I would suggest that @ottobackwards or @chrisdutz should do a final review then and merge et, as im not confident enough in the c-world :D

@thomas169
Copy link
Member Author

its done in relation to the title. But any commits to my fork also come into this PR, hence the later 3 commits.

@chrisdutz chrisdutz merged commit e2944d3 into apache:develop May 3, 2021
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.

3 participants