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

feat: Add bigEndian() and littleEndian() fields to JByteOrder #93

Closed
wants to merge 1 commit into from

Conversation

mrousavy
Copy link
Contributor

@mrousavy mrousavy commented Jan 25, 2024

Motivation

JByteBuffers could only be initialized with nativeOrder endianness.
With this PR, both littleEndian() and bigEndian() fields are exposed in JByteOrder and can now be set through JByteBuffer::order(...).

Summary

Get static fields LITTLE_ENDIAN and BIG_ENDIAN through JByteOrder.

Test Plan

Test code:

Default:

auto buffer = JByteBuffer::allocateDirect(5);
uint8_t* data = buffer->getDirectData();
data[0] = 255;
data[1] = 1;
__android_log_print(ANDROID_LOG_INFO, "Test", "First item: %i", data[0]);

With custom Endian:

auto buffer = JByteBuffer::allocateDirect(5);
buffer->order(JByteOrder::bigEndian()); // <-- or littleEndian()
uint8_t* data = buffer->getDirectData();
data[0] = 255;
data[1] = 1;
__android_log_print(ANDROID_LOG_INFO, "Test", "First item: %i", data[0]);

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 25, 2024
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mrousavy
Copy link
Contributor Author

What's the linter complaining about? Probably the long line in littleEndian()? Can I run this locally?

@cortinico
Copy link
Contributor

What's the linter complaining about? Probably the long line in littleEndian()? Can I run this locally?

It's a clang formatting issue. I've reformatted it for you and scheduled a land.

Will need also a new release of fbjni if you need to use this.
If you could do the bump to NDK 26.1 similarly to how I did on facebook/react-native#42656 would be extremely helpful 👍 so we can get a new release out soon

@mrousavy
Copy link
Contributor Author

Thanks!
& sure, I'll do that tmrw :)

@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in 0454714.

@mrousavy
Copy link
Contributor Author

Done: #94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants