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

PLC4X-57 Bugfix #25

Merged
merged 2 commits into from Oct 2, 2018
Merged

Conversation

JulianFeinauer
Copy link
Contributor

Fixed PLC4X-57.
Also fixed Output from HelloPlc4x.java.
Fixed some small bugs.
Added TODO for returning arrays on bit streams and chars / strings.

@JulianFeinauer
Copy link
Contributor Author

Sorry, the rebase did not work and I had to do it in a second commit.
@chrisdutz I added the functionality to return arrays in the Plx4XS7Protocol.java and it would be good if you look over it and my todo there.
Be aware, the code is pretty ugly due to some Type Erasure problems (Generic Arrays), especially in the bit-string case (but it was ugly there before so ... :D).

switch (field.getDataType()) {
// -----------------------------------------
// Bit
// -----------------------------------------
case BOOL: {
byte byteValue = data.readByte();
fieldItem = new S7BooleanFieldItem(field.getDataType(),byteValue != 0x00);
Boolean[] booleans = readAllValues(field, i -> data.readByte() != 0x00).toArray(new Boolean[field.getNumElements()]);
Copy link
Contributor

Choose a reason for hiding this comment

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

actually sonar finds a presized array as a bug: see here https://stackoverflow.com/a/29444594/850036

Copy link
Contributor

Choose a reason for hiding this comment

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

review...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm about to fix it... didn't know that

Also fixed Output from HelloPlc4x.java.
Fixed some small bugs.
Added TODO for returning arrays on bit streams and chars / strings.
Fixed Presized Arrays.
BitSet bitSet = BitSet.valueOf(new byte[]{data.readByte()});
Boolean[] booleanValues = new Boolean[8];
for(int i = 0; i < 8; i++) {
Byte[] bytes = readAllValues(field, i -> data.readByte()).toArray(new Byte[field.getNumElements()]);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should check data if we can read this amount of bytes. This could maybe be done generic if we pass the data variable and the method reference: data::readByte or something like that.

switch (field.getDataType()) {
// -----------------------------------------
// Bit
// -----------------------------------------
case BOOL: {
byte byteValue = data.readByte();
fieldItem = new S7BooleanFieldItem(field.getDataType(),byteValue != 0x00);
Boolean[] booleans = readAllValues(field, i -> data.readByte() != 0x00).toArray(new Boolean[field.getNumElements()]);
Copy link
Contributor

Choose a reason for hiding this comment

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

review...

Used more of sebastians generics magic to remove ".toArray(...)".
@chrisdutz
Copy link
Contributor

By the way ... looking good ... just had a few minutes after lunch and checked out your changes.
And I already added a test for the field with array with a more than one digit number of elements in a separate test case.

@JulianFeinauer JulianFeinauer merged commit ae9db03 into apache:master Oct 2, 2018
@JulianFeinauer JulianFeinauer deleted the new-s7-parser branch October 2, 2018 07:50
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.

None yet

3 participants