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

Add support for large messages #54

Merged
merged 2 commits into from
Feb 5, 2021
Merged

Add support for large messages #54

merged 2 commits into from
Feb 5, 2021

Conversation

bpg
Copy link
Contributor

@bpg bpg commented Jan 2, 2021

  • Fix implicit type conversion in reader that caused byte overflow when number of fields in definition message in >85.
  • Add real life fit activity to the test suite to replicate the problem
  • Fix definition message creation logic in writer -- now it scans all records of specific type to find all fields that have valid values to build a definition message.

This fixes #43, and fixes #53

- Fix implicit type conversion in reader that caused byte overflow when number of fields in definition message in >85.
- Add real life fit activity to the test quite to replicate the problem
- Fix definition message creation logic in `writer` -- now it scans all records of specific type to find all fields that have valid values to build a definition message.

Fixes #43 and #53
@tormoder
Copy link
Owner

tormoder commented Jan 4, 2021

@bpg Thanks for digging into this.

I'll try to take a look as soon as possible.

@tormoder
Copy link
Owner

tormoder commented Feb 2, 2021

Sorry for the delay...

Thanks for spotting the overflow.

Suggestion, could we just remove the maxFieldSize constant?
And just set the array length for tmp to 255 * 3 directly?

@bpg
Copy link
Contributor Author

bpg commented Feb 5, 2021

Thanks, updated!

@tormoder tormoder merged commit 5871495 into tormoder:master Feb 5, 2021
@bpg bpg deleted the large-message-support branch February 5, 2021 15:13
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.

Writer excludes fields that are missing from the first data message strange error message
2 participants