Skip to content

Commit

Permalink
Remove usage of GetDevice() API in Android controller (#10612)
Browse files Browse the repository at this point in the history 
* Reduce usage of GetDevice() API in Android controller

* Set PeerId correctly

* remove remaining use of GetDevice(), GetCHIPDevice() and getDevicePointer()

* add suspend

* more build fixes
  • Loading branch information
@pan-apple
pan-apple authored Oct 18, 2021
1 parent ca22841 commit 05c3a35
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 68 deletions.
28 changes: 17 additions & 11 deletions .../CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/EnterNetworkFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,18 @@ import kotlinx.android.synthetic.main.enter_thread_network_fragment.xpanIdEd
import kotlinx.android.synthetic.main.enter_wifi_network_fragment.pwdEd
import kotlinx.android.synthetic.main.enter_wifi_network_fragment.ssidEd
import kotlinx.android.synthetic.main.enter_wifi_network_fragment.view.saveNetworkBtn
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch

/**
* Fragment to collect Wi-Fi network information from user and send it to device being provisioned.
*/
class EnterNetworkFragment : Fragment() {

private val scope = CoroutineScope(Dispatchers.Main + Job())

private val networkType: ProvisionNetworkType
get() = requireNotNull(
ProvisionNetworkType.fromName(arguments?.getString(ARG_PROVISION_NETWORK_TYPE))
Expand All @@ -58,7 +64,7 @@ class EnterNetworkFragment : Fragment() {
}

if (USE_HARDCODED_WIFI) {
saveHardcodedWifiNetwork()
scope.launch { saveHardcodedWifiNetwork() }
}

return inflater.inflate(layoutRes, container, false).apply {
Expand All @@ -68,17 +74,17 @@ class EnterNetworkFragment : Fragment() {

private fun onSaveNetworkClicked() {
if (networkType == ProvisionNetworkType.WIFI) {
saveWifiNetwork()
scope.launch { saveWifiNetwork() }
} else {
saveThreadNetwork()
scope.launch { saveThreadNetwork() }
}
}

private fun saveHardcodedWifiNetwork() {
private suspend fun saveHardcodedWifiNetwork() {
addAndEnableWifiNetwork(HARDCODED_WIFI_SSID, HARDCODED_WIFI_PASSWORD)
}

private fun saveWifiNetwork() {
private suspend fun saveWifiNetwork() {
val ssid = ssidEd?.text
val pwd = pwdEd?.text

Expand All @@ -90,13 +96,13 @@ class EnterNetworkFragment : Fragment() {
addAndEnableWifiNetwork(ssid.toString(), pwd.toString())
}

private fun addAndEnableWifiNetwork(ssid: String, password: String) {
private suspend fun addAndEnableWifiNetwork(ssid: String, password: String) {
// Uses UTF-8 as default
val ssidBytes = ssid.toByteArray()
val pwdBytes = password.toByteArray()

val devicePtr = ChipClient.getDeviceController(requireContext())
.getDevicePointer(DeviceIdUtil.getLastDeviceId(requireContext()))
val devicePtr =
ChipClient.getConnectedDevicePointer(requireContext(), DeviceIdUtil.getLastDeviceId(requireContext()))
val cluster = NetworkCommissioningCluster(devicePtr, /* endpointId = */ 0)

val enableNetworkCallback = object :
Expand Down Expand Up @@ -150,7 +156,7 @@ class EnterNetworkFragment : Fragment() {
}, ssidBytes, pwdBytes, /* breadcrumb = */ 0L, ADD_NETWORK_TIMEOUT)
}

private fun saveThreadNetwork() {
private suspend fun saveThreadNetwork() {
val channelStr = channelEd.text
val panIdStr = panIdEd.text

Expand Down Expand Up @@ -186,8 +192,8 @@ class EnterNetworkFragment : Fragment() {
return
}

val devicePtr = ChipClient.getDeviceController(requireContext())
.getDevicePointer(DeviceIdUtil.getLastDeviceId(requireContext()))
val devicePtr =
ChipClient.getConnectedDevicePointer(requireContext(), DeviceIdUtil.getLastDeviceId(requireContext()))
val cluster = NetworkCommissioningCluster(devicePtr, /* endpointId = */ 0)

val operationalDataset = makeThreadOperationalDataset(
Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate,

virtual void ReleaseDevice(Device * device);

void ReleaseDeviceById(NodeId remoteDeviceId);

#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
void RegisterDeviceAddressUpdateDelegate(DeviceAddressUpdateDelegate * delegate) { mDeviceAddressUpdateDelegate = delegate; }
void RegisterDeviceDiscoveryDelegate(DeviceDiscoveryDelegate * delegate) { mDeviceDiscoveryDelegate = delegate; }
Expand Down Expand Up @@ -305,7 +307,6 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate,
uint16_t FindDeviceIndex(SessionHandle session);
uint16_t FindDeviceIndex(NodeId id);
void ReleaseDevice(uint16_t index);
void ReleaseDeviceById(NodeId remoteDeviceId);
CHIP_ERROR InitializePairedDeviceList();
CHIP_ERROR SetPairedDeviceList(ByteSpan pairedDeviceSerializedSet);
ControllerDeviceInitParams GetControllerDeviceInitParams();
Expand Down
58 changes: 13 additions & 45 deletions src/controller/java/CHIPDeviceController-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ using namespace chip::Controller;

#define CDC_JNI_CALLBACK_LOCAL_REF_COUNT 256

static void GetCHIPDevice(JNIEnv * env, long wrapperHandle, uint64_t deviceId, Device ** device);
static void ThrowError(JNIEnv * env, CHIP_ERROR errToThrow);
static void * IOThreadMain(void * arg);
static CHIP_ERROR N2J_Error(JNIEnv * env, CHIP_ERROR inErr, jthrowable & outEx);
Expand Down Expand Up @@ -275,19 +274,6 @@ JNI_METHOD(void, stopDevicePairing)(JNIEnv * env, jobject self, jlong handle, jl
}
}

JNI_METHOD(jlong, getDevicePointer)(JNIEnv * env, jobject self, jlong handle, jlong deviceId)
{
chip::DeviceLayer::StackLock lock;
Device * chipDevice = nullptr;

ChipLogProgress(Controller, "getDevicePointer() called with device ID");

GetCHIPDevice(env, handle, deviceId, &chipDevice);

static_assert(sizeof(jlong) >= sizeof(void *), "Need to store a pointer in a Java handle");
return reinterpret_cast<jlong>(chipDevice);
}

JNI_METHOD(void, getConnectedDevicePointer)(JNIEnv * env, jobject self, jlong handle, jlong nodeId, jlong callbackHandle)
{
chip::DeviceLayer::StackLock lock;
Expand Down Expand Up @@ -326,20 +312,9 @@ JNI_METHOD(void, disconnectDevice)(JNIEnv * env, jobject self, jlong handle, jlo
{
chip::DeviceLayer::StackLock lock;
AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(handle);
CHIP_ERROR err = CHIP_NO_ERROR;
Device * chipDevice = nullptr;

ChipLogProgress(Controller, "disconnectDevice() called with deviceId");

err = wrapper->Controller()->GetDevice(deviceId, &chipDevice);

if (err != CHIP_NO_ERROR || !chipDevice)
{
ChipLogError(Controller, "Failed to get paired device.");
ThrowError(env, err);
}

wrapper->Controller()->ReleaseDevice(chipDevice);
wrapper->Controller()->ReleaseDeviceById(deviceId);
}

JNI_METHOD(jboolean, isActive)(JNIEnv * env, jobject self, jlong handle)
Expand All @@ -350,33 +325,26 @@ JNI_METHOD(jboolean, isActive)(JNIEnv * env, jobject self, jlong handle)
return chipDevice->IsActive();
}

void GetCHIPDevice(JNIEnv * env, long wrapperHandle, uint64_t deviceId, Device ** chipDevice)
{
AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(wrapperHandle);
CHIP_ERROR err = CHIP_NO_ERROR;

err = wrapper->Controller()->GetDevice(deviceId, chipDevice);

if (err != CHIP_NO_ERROR || !chipDevice)
{
ChipLogError(Controller, "Failed to get paired device.");
ThrowError(env, err);
}
}

JNI_METHOD(jstring, getIpAddress)(JNIEnv * env, jobject self, jlong handle, jlong deviceId)
{
chip::DeviceLayer::StackLock lock;
Device * chipDevice = nullptr;

GetCHIPDevice(env, handle, deviceId, &chipDevice);
AndroidDeviceControllerWrapper * wrapper = AndroidDeviceControllerWrapper::FromJNIHandle(handle);

chip::Inet::IPAddress addr;
uint16_t port;
char addrStr[50];

if (!chipDevice->GetAddress(addr, port))
return nullptr;
CHIP_ERROR err =
wrapper->Controller()->GetPeerAddressAndPort(PeerId()
.SetCompressedFabricId(wrapper->Controller()->GetCompressedFabricId())
.SetNodeId(static_cast<chip::NodeId>(deviceId)),
addr, port);

if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed to get device address.");
ThrowError(env, err);
}

addr.ToString(addrStr);
return env->NewStringUTF(addrStr);
Expand Down
14 changes: 3 additions & 11 deletions src/controller/java/src/chip/devicecontroller/ChipDeviceController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,11 @@ public void pairTestDeviceWithoutSecurity(String ipAddress) {
}

/**
* Returns a pointer to a device with the specified nodeId. The device is not guaranteed to be
* connected.
* Through GetConnectedDeviceCallback, returns a pointer to a connected device or an error.
*
* <p>TODO(#8443): This method and getConnectedDevicePointer() could benefit from ChipDevice
* abstraction to hide the pointer passing.
* <p>TODO(#8443): This method could benefit from a ChipDevice abstraction to hide the pointer
* passing.
*/
public long getDevicePointer(long nodeId) {
return getDevicePointer(deviceControllerPtr, nodeId);
}

/** Through GetConnectedDeviceCallback, returns a pointer to a connected device or an error. */
public void getConnectedDevicePointer(long nodeId, GetConnectedDeviceCallback callback) {
GetConnectedDeviceCallbackJni jniCallback = new GetConnectedDeviceCallbackJni(callback);
getConnectedDevicePointer(deviceControllerPtr, nodeId, jniCallback.getCallbackHandle());
Expand Down Expand Up @@ -240,8 +234,6 @@ private native void pairDeviceWithAddress(

private native void unpairDevice(long deviceControllerPtr, long deviceId);

private native long getDevicePointer(long deviceControllerPtr, long deviceId);

private native void getConnectedDevicePointer(
long deviceControllerPtr, long deviceId, long callbackHandle);

Expand Down

0 comments on commit 05c3a35

Please sign in to comment.