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 string encoded frame length header #84

Merged
merged 1 commit into from
Apr 14, 2020
Merged

Add support for string encoded frame length header #84

merged 1 commit into from
Apr 14, 2020

Conversation

solnaranu
Copy link
Contributor

@solnaranu solnaranu commented Apr 9, 2020

Hello @kpavlov and other contributors,

first, let me thank you for this library. I just started using it and it looks like it's going to help me save a lot of time.

Now, to the issue I had - I'm integrating with a vendor which doesn't use a binary frame length header, but a 4 byte, zero padded, ASCII encoded one. For example, an ISO message of length 93 would have the length header of 0093.

Since the first stage in the ChannelPipeline is Netty's LengthFieldBasedFrameDecoder and it expects a binary length header, I needed to extend it and override it's getUnadjustedFrameLength method in StringLengthFieldBasedFrameDecoder.

Then, I did some config plumbing in ConnectorConfiguration which is used in Iso8583Encoder and Iso8583ChannelInitializer.

Currently, US_ASCII is hardcoded when encoding and decoding, but I'd be willing to make that configurable if it's a realistic use case, which I'm not aware of.

Usage example for the new configuration:

val serverConfiguration = ServerConfiguration.newBuilder()
        .frameLengthFieldLength(4)
        .encodeFrameLengthAsString(true)
        .build

Existing tests are passing, but I haven't added new tests, only tested it manually. I will add tests if this PR has a chance of being accepted. I'd be glad to discuss it and take suggestions.

Cheers,
Bojan

@kpavlov
Copy link
Owner

kpavlov commented Apr 14, 2020

Thank you, @solnaranu for your PR. Please add some tests.

@kpavlov kpavlov merged commit 5943f3e into kpavlov:master Apr 14, 2020
@solnaranu
Copy link
Contributor Author

Thank you, @solnaranu for your PR. Please add some tests.

Will do! I'll try to find some time in the following weeks.

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