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.

Sign up for GitHub

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

Jump to bottom

Feature/nifi integration type mapping #752

Merged
merged 4 commits into from
Jan 20, 2023
Merged

Feature/nifi integration type mapping #752

merged 4 commits into from
Jan 20, 2023

Conversation

QuanticPony
Copy link
Contributor

Improvements in type mapping from Plc4x to Avro. Added unit test for record source processor.

Changes in mapping:

  • PlcBYTE: string -> bytes
  • PlcLINT: string -> long
  • PlcREAL: double -> float
  • PlcLREAL: string -> double

General improvements in org.apache.plc4x.nifi.util.Plc4xCommon:

  • Reduced repeated code
  • Updated mapping table in readme
  • Fixed: reading PlcList not working
  • Fixed: PlcByte to Avro mapping was set to bool, caused exception, changed to bytes

@QuanticPony
fix(plc4j/integrations/apache-nifi): Update mapping
c1560dd 
between PlcType and Avro types.

* Update PlcType to Avro type mapping
* Added unit test for Plc4xSourceRecordProcessor
* Added common test utils.
@QuanticPony
fix(plc4j/integrations/apache-nifi): Add license
5600472 
to Plc4xCommonTest
@QuanticPony
Merge remote-tracking branch 'upstream/develop' into feature/nifi-int…
a406381 
…egration-type-mapping
@QuanticPony QuanticPony marked this pull request as ready for review January 18, 2023 14:16
ottobackwards
ottobackwards requested changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@ottobackwards ottobackwards left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This is really nice, only the one comment on the type in the doc being 'bytes'

plc4j/integrations/apache-nifi/README.md
| PlcBigDecimal | float |
| PlcBigInteger | long |
| PlcBitString | string |
|----------:|-----------|
| PlcBOOL | boolean |
Copy link
Contributor

Choose a reason for hiding this comment

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

should be byte[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Avro specification (https://avro.apache.org/docs/1.11.1/specification/#primitive-types) the primitive type is called bytes

@QuanticPony
Update readme
fec253d 
* Add explicit source of Avro types
* Change output of example to actual output
ottobackwards
ottobackwards approved these changes Jan 20, 2023
View reviewed changes
Copy link
Contributor

@ottobackwards ottobackwards left a comment

Choose a reason for hiding this comment

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

Thanks, sorry for my misunderstanding.

@ottobackwards ottobackwards merged commit 91bf9fa into apache:develop Jan 20, 2023
@QuanticPony QuanticPony deleted the feature/nifi-integration-type-mapping branch March 1, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ottobackwards ottobackwards ottobackwards approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@QuanticPony @ottobackwards