First JNI test & changes necessary to make it work #24355
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
gmarcosb
commented
Jan 10, 2023
gmarcosb
commented
- Add first JNI unit test, GetConnectedDeviceCallbackJniTest, testing success/failure cases for device callback
- Change various references of "SessionHandle &" to instead use "cont SessionHandle &" to match how SessionHandle is often used as a parameter & to support rvalue
- Add Java/JNI hooks into for-test classes (like MessagingContext) to allow for unit tests of Java & JNI functionality
- Use commonly-defined ConfigurationMgrImpl() extern function (which must be implemented ) instead of ConfigurationManagerImpl::GetDefaultInstance() which is only caught at compile-time
…t SessionHandle &" to match how SessionHandle is often used as a parameter & to support rvalue * Add Java/JNI hooks into for-test classes (like MessagingContext) to allow for unit tests of Java & JNI functionality * Add first JNI unit test, GetConnectedDeviceCallbackJniTest, testing success/failure cases for device callback
pullapprove
bot
requested review from
andy31415,
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jepenven-silabs,
jmartinez-silabs and
jtung-apple
|
PR #24355: Size comparison from e7acdf7 to 5dbe3be Increases (14 builds for bl702, cc13x2_26x2, nrfconnect, psoc6, qpg, telink)
Decreases (6 builds for bl602, cc13x2_26x2, qpg, telink)
Full report (41 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
bzbarsky-apple
approved these changes
Jan 12, 2023
|
PR #24355: Size comparison from ff8fe37 to f1894aa Increases (11 builds for bl602, bl702, cc13x2_26x2, nrfconnect, psoc6, telink)
Decreases (11 builds for bl702, cc13x2_26x2, esp32, linux, psoc6, telink)
Full report (54 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #24355: Size comparison from 3670731 to 03a7ecb Increases (9 builds for bl702, cc13x2_26x2, psoc6, telink)
Decreases (4 builds for bl702, cc13x2_26x2, psoc6)
Full report (36 builds for bl602, bl702, cc13x2_26x2, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #24355: Size comparison from 3670731 to d3fc5dc Increases (12 builds for bl602, bl702, cc13x2_26x2, psoc6, telink)
Decreases (10 builds for cc13x2_26x2, esp32, k32w, linux, psoc6, telink)
Full report (54 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #24355: Size comparison from b36439e to 9cb8b16 Increases (13 builds for bl702, cc13x2_26x2, esp32, k32w, nrfconnect, psoc6, qpg, telink)
Decreases (5 builds for bl602, cc13x2_26x2, linux, telink)
Full report (54 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #24355: Size comparison from a25af90 to ef0fd01 Increases (10 builds for bl602, bl702, cc13x2_26x2, psoc6, telink)
Decreases (9 builds for bl702, cc13x2_26x2, esp32, linux, psoc6, telink)
Full report (54 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
@gmarcosb Note that it breaks the build on master. |
andy31415
added a commit
to andy31415/connectedhomeip
that referenced
this pull request
Jan 16, 2023
andy31415
added a commit
that referenced
this pull request
Jan 16, 2023
|
Thanks @andy31415! |
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
* * Change various references of "SessionHandle &" to instead use "const SessionHandle &" to match how SessionHandle is often used as a parameter & to support rvalue * Add Java/JNI hooks into for-test classes (like MessagingContext) to allow for unit tests of Java & JNI functionality * Add first JNI unit test, GetConnectedDeviceCallbackJniTest, testing success/failure cases for device callback * Restyled by whitespace * Restyled by clang-format * Restyled by gn * * Fix a user-reported bug where we were seeing the following callstack: JNIEnv::NewObject(_jclass*, _jmethodID*, ...) chip::Controller::GetConnectedDeviceCallback::OnDeviceConnectionFailureFn(void*, chip::ScopedNodeId const&, chip::ChipError) chip::OperationalSessionSetup::DequeueConnectionCallbacks(chip::ChipError) chip::OperationalSessionSetup::OnNodeAddressResolutionFailed(chip::PeerId const&, chip::ChipError) chip::AddressResolve::Impl::Resolver::HandleAction(chip::IntrusiveList<chip::AddressResolve::Impl::NodeLookupHandle, (chip::IntrusiveMode)0, chip::IntrusiveListBaseHook<chip::AddressResolve::Impl::NodeLookupHandle, (chip::IntrusiveMode)0> >::Iterator&) chip::AddressResolve::Impl::Resolver::HandleTimer() chip::System::LayerImplSelect::HandleEvents() Though JNI tests were added to try to catch this at presubmit, those tests did not fail, which leads me to believe there's a difference in the JRE (wherein the version with the user-reported bug), a cast to an int is not being done, whereas in android emulator, the cast to int is done * Add a README pointing to building android guide & specifying that these tests must be run externally due to android emulator dependency * Fix path to android_building.md * Restyled by clang-format * Restyled by prettier-markdown * Missed const correctness changes for SessionHandle& * Restyled by clang-format * More missed const correctness changes for SessionHandle& * Restyled by clang-format * More missed const correctness changes for SessionHandle& * Restyled by clang-format * Only build JNI test libs if chip_link_tests * Restyled by gn * Add missing tests.gni import Co-authored-by: Restyled.io <commits@restyled.io>
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.