Skip to content

Commit

Permalink
Allow UDP listening port selection for Android (#19596)
Browse files Browse the repository at this point in the history
* Allow UDP listening port selection for Android

* Restyled by google-java-format

* Restyled by clang-format

Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
gmarcosb and restyled-commits authored Jun 23, 2022
1 parent c4ebb05 commit fd2e975
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 38 deletions.
4 changes: 2 additions & 2 deletions src/controller/java/AndroidDeviceControllerWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(
chip::Inet::EndPointManager<Inet::TCPEndPoint> * tcpEndPointManager,
chip::Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager, AndroidOperationalCredentialsIssuerPtr opCredsIssuerPtr,
jobject keypairDelegate, jbyteArray rootCertificate, jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate,
jbyteArray ipkEpochKey, CHIP_ERROR * errInfoOnFailure)
jbyteArray ipkEpochKey, uint16_t listenPort, CHIP_ERROR * errInfoOnFailure)
{
if (errInfoOnFailure == nullptr)
{
Expand Down Expand Up @@ -140,7 +140,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew(
#if CONFIG_NETWORK_LAYER_BLE
initParams.bleLayer = DeviceLayer::ConnectivityMgr().GetBleLayer();
#endif
initParams.listenPort = CHIP_PORT + 1;
initParams.listenPort = listenPort;
setupParams.pairingDelegate = wrapper.get();
setupParams.operationalCredentialsDelegate = opCredsIssuer;
initParams.fabricIndependentStorage = wrapper.get();
Expand Down
3 changes: 2 additions & 1 deletion src/controller/java/AndroidDeviceControllerWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
* @param[in] intermediateCertificate an X.509 DER-encoded intermediate certificate for this node
* @param[in] nodeOperationalCertificate an X.509 DER-encoded operational certificate for this node
* @param[in] ipkEpochKey the IPK epoch key to use for this node
* @param[in] listenPort the UDP port to listen on
* @param[out] errInfoOnFailure a pointer to a CHIP_ERROR that will be populated if this method returns nullptr
*/
static AndroidDeviceControllerWrapper * AllocateNew(JavaVM * vm, jobject deviceControllerObj, chip::NodeId nodeId,
Expand All @@ -120,7 +121,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel
AndroidOperationalCredentialsIssuerPtr opCredsIssuer,
jobject keypairDelegate, jbyteArray rootCertificate,
jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate,
jbyteArray ipkEpochKey, CHIP_ERROR * errInfoOnFailure);
jbyteArray ipkEpochKey, uint16_t listenPort, CHIP_ERROR * errInfoOnFailure);

private:
using ChipDeviceControllerPtr = std::unique_ptr<chip::Controller::DeviceCommissioner>;
Expand Down
1 change: 1 addition & 0 deletions src/controller/java/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ android_library("java") {
"src/chip/devicecontroller/ChipCommandType.java",
"src/chip/devicecontroller/ChipDeviceController.java",
"src/chip/devicecontroller/ChipDeviceControllerException.java",
"src/chip/devicecontroller/ControllerParams.java",
"src/chip/devicecontroller/DiscoveredDevice.java",
"src/chip/devicecontroller/GetConnectedDeviceCallbackJni.java",
"src/chip/devicecontroller/KeypairDelegate.java",
Expand Down
52 changes: 43 additions & 9 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ pthread_t sIOThread = PTHREAD_NULL;

jclass sChipDeviceControllerExceptionCls = NULL;

const char * PARAMS_CLASS = "()Lchip/devicecontroller/ControllerParams;";

} // namespace

// NOTE: Remote device ID is in sync with the echo server device id
Expand Down Expand Up @@ -153,24 +155,56 @@ void JNI_OnUnload(JavaVM * jvm, void * reserved)
chip::Platform::MemoryShutdown();
}

JNI_METHOD(jlong, newDeviceController)
(JNIEnv * env, jobject self, jobject keypairDelegate, jbyteArray rootCertificate, jbyteArray intermediateCertificate,
jbyteArray operationalCertificate, jbyteArray ipk)
JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject controllerParams)
{
chip::DeviceLayer::StackLock lock;
CHIP_ERROR err = CHIP_NO_ERROR;
AndroidDeviceControllerWrapper * wrapper = NULL;
long result = 0;

ChipLogProgress(Controller, "newDeviceController() called");
std::unique_ptr<chip::Controller::AndroidOperationalCredentialsIssuer> opCredsIssuer(
new chip::Controller::AndroidOperationalCredentialsIssuer());
wrapper = AndroidDeviceControllerWrapper::AllocateNew(
sJVM, self, kLocalDeviceId, chip::kUndefinedCATs, &DeviceLayer::SystemLayer(), DeviceLayer::TCPEndPointManager(),
DeviceLayer::UDPEndPointManager(), std::move(opCredsIssuer), keypairDelegate, rootCertificate, intermediateCertificate,
operationalCertificate, ipk, &err);

// Retrieve initialization params.
jmethodID getUdpListenPort;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getUdpListenPort", PARAMS_CLASS, &getUdpListenPort);

This comment has been minimized.

Copy link
@robertfarnum

robertfarnum Jun 23, 2022

Contributor

PARAMS_CLASS is not the proper signature of "getUdpListenPort" or any of the subsequent FindMethod() calls.

SuccessOrExit(err);

jmethodID getKeypairDelegate;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getKeypairDelegate", PARAMS_CLASS,
&getKeypairDelegate);

jmethodID getRootCertificate;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getRootCertificate", PARAMS_CLASS,
&getRootCertificate);

jmethodID getIntermediateCertificate;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getIntermediateCertificate", PARAMS_CLASS,
&getIntermediateCertificate);

jmethodID getOperationalCertificate;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getOperationalCertificate", PARAMS_CLASS,
&getOperationalCertificate);

jmethodID getIpk;
err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getIpk", PARAMS_CLASS, &getIpk);

{
uint16_t listenPort = env->CallIntMethod(controllerParams, getUdpListenPort);
jobject keypairDelegate = env->CallObjectMethod(controllerParams, getKeypairDelegate);
jbyteArray rootCertificate = (jbyteArray) env->CallObjectMethod(controllerParams, getRootCertificate);
jbyteArray intermediateCertificate = (jbyteArray) env->CallObjectMethod(controllerParams, getIntermediateCertificate);
jbyteArray operationalCertificate = (jbyteArray) env->CallObjectMethod(controllerParams, getOperationalCertificate);
jbyteArray ipk = (jbyteArray) env->CallObjectMethod(controllerParams, getIpk);

std::unique_ptr<chip::Controller::AndroidOperationalCredentialsIssuer> opCredsIssuer(
new chip::Controller::AndroidOperationalCredentialsIssuer());
wrapper = AndroidDeviceControllerWrapper::AllocateNew(
sJVM, self, kLocalDeviceId, chip::kUndefinedCATs, &DeviceLayer::SystemLayer(), DeviceLayer::TCPEndPointManager(),
DeviceLayer::UDPEndPointManager(), std::move(opCredsIssuer), keypairDelegate, rootCertificate, intermediateCertificate,
operationalCertificate, ipk, listenPort, &err);
SuccessOrExit(err);
}

// Create and start the IO thread. Must be called after Controller()->Init
if (sIOThread == PTHREAD_NULL)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,14 @@ public static void loadJni() {
return;
}

/**
* Returns a new {@link ChipDeviceController} with ephemerally generated operational credentials.
*/
/** Returns a new {@link ChipDeviceController} with default parameters. */
public ChipDeviceController() {
deviceControllerPtr = newDeviceController();
this(ControllerParams.newBuilder().build());
}

/**
* Returns a new {@link ChipDeviceController} which uses the provided {@code operationalKeyConfig}
* as its operating credentials.
*/
public ChipDeviceController(OperationalKeyConfig operationalKeyConfig) {
deviceControllerPtr =
newDeviceController(
operationalKeyConfig.getKeypairDelegate(),
operationalKeyConfig.getTrustedRootCertificate(),
operationalKeyConfig.getIntermediateCertificate(),
operationalKeyConfig.getNodeOperationalCertificate(),
operationalKeyConfig.getIpkEpochKey());
/** Returns a new {@link ChipDeviceController} with the specified parameters. */
public ChipDeviceController(ControllerParams params) {
deviceControllerPtr = newDeviceController(params);
}

public void setCompletionListener(CompletionListener listener) {
Expand Down Expand Up @@ -431,16 +420,7 @@ public native void readPath(
long devicePtr,
List<ChipAttributePath> attributePaths);

private long newDeviceController() {
return newDeviceController(null, null, null, null, null);
}

private native long newDeviceController(
@Nullable KeypairDelegate keypairDelegate,
@Nullable byte[] rootCertificate,
@Nullable byte[] intermediateCertificate,
@Nullable byte[] operationalCertificate,
@Nullable byte[] ipk);
private native long newDeviceController(ControllerParams params);

private native void pairDevice(
long deviceControllerPtr,
Expand Down
118 changes: 118 additions & 0 deletions src/controller/java/src/chip/devicecontroller/ControllerParams.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package chip.devicecontroller;

import androidx.annotation.Nullable;

/** Parameters representing initialization arguments for {@link ChipDeviceController}. */
public final class ControllerParams {

private final int udpListenPort;
@Nullable private final KeypairDelegate keypairDelegate;
@Nullable private final byte[] rootCertificate;
@Nullable private final byte[] intermediateCertificate;
@Nullable private final byte[] operationalCertificate;
@Nullable private final byte[] ipk;

private static final int LEGACY_GLOBAL_CHIP_PORT = 5540;

/** @param udpListenPort the UDP listening port, or 0 to pick any available port. */
private ControllerParams(Builder builder) {
this.udpListenPort = builder.udpListenPort;
this.keypairDelegate = builder.keypairDelegate;
this.rootCertificate = builder.rootCertificate;
this.intermediateCertificate = builder.intermediateCertificate;
this.operationalCertificate = builder.operationalCertificate;
this.ipk = builder.ipk;
}

/** Gets the UDP listening port; 0 indicates "any available port" */
public int getUdpListenPort() {
return udpListenPort;
}

public KeypairDelegate getKeypairDelegate() {
return keypairDelegate;
}

public byte[] getRootCertificate() {
return rootCertificate;
}

public byte[] getIntermediateCertificate() {
return intermediateCertificate;
}

public byte[] getOperationalCertificate() {
return operationalCertificate;
}

public byte[] getIpk() {
return ipk;
}

/** Returns parameters with ephemerally generated operational credentials */
public static Builder newBuilder() {
return new Builder();
}

/**
* Returns parameters which uses the provided {@code operationalKeyConfig} as its operating
* credentials.
*/
public static Builder newBuilder(OperationalKeyConfig operationalKeyConfig) {
return newBuilder()
.setKeypairDelegate(operationalKeyConfig.getKeypairDelegate())
.setRootCertificate(operationalKeyConfig.getTrustedRootCertificate())
.setIntermediateCertificate(operationalKeyConfig.getIntermediateCertificate())
.setOperationalCertificate(operationalKeyConfig.getNodeOperationalCertificate())
.setIpk(operationalKeyConfig.getIpkEpochKey());
}

/** Builder for {@link ControllerParams}. */
public static class Builder {
private int udpListenPort = LEGACY_GLOBAL_CHIP_PORT + 1;
@Nullable private KeypairDelegate keypairDelegate = null;
@Nullable private byte[] rootCertificate = null;
@Nullable private byte[] intermediateCertificate = null;
@Nullable private byte[] operationalCertificate = null;
@Nullable private byte[] ipk = null;

private Builder() {}

public Builder setUdpListenPort(int udpListenPort) {
if (udpListenPort < 0) {
throw new IllegalArgumentException("udpListenPort must be >= 0");
}
this.udpListenPort = udpListenPort;
return this;
}

public Builder setKeypairDelegate(KeypairDelegate keypairDelegate) {
this.keypairDelegate = keypairDelegate;
return this;
}

public Builder setRootCertificate(byte[] rootCertificate) {
this.rootCertificate = rootCertificate;
return this;
}

public Builder setIntermediateCertificate(byte[] intermediateCertificate) {
this.intermediateCertificate = intermediateCertificate;
return this;
}

public Builder setOperationalCertificate(byte[] operationalCertificate) {
this.operationalCertificate = operationalCertificate;
return this;
}

public Builder setIpk(byte[] ipk) {
this.ipk = ipk;
return this;
}

public ControllerParams build() {
return new ControllerParams(this);
}
}
}

4 comments on commit fd2e975

@pakls
Copy link
Contributor

@pakls pakls commented on fd2e975 Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Marcos,

I'm seeing error finding getUdpListenPort during newDeviceController().
Not sure is this only me or others having the same issue.

This happened when I run CHIPTool and click 'Discover' in AddressCommissioningFragment.
Could you help sheding some light how this can be fixed? Thanks.

@pakls
Copy link
Contributor

@pakls pakls commented on fd2e975 Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JniReferences::FindMethod() can't find a method here:

*methodId = env->GetMethodID(javaClass, methodName, methodSignature);
VerifyOrReturnError(*methodId != nullptr, CHIP_JNI_ERROR_METHOD_NOT_FOUND);

@pakls
Copy link
Contributor

@pakls pakls commented on fd2e975 Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some try and error, I think this can find methods correctly:

> -    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getUdpListenPort", PARAMS_CLASS, &getUdpListenPort);
> +    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getUdpListenPort", "()I",
> +                                                       &getUdpListenPort);
> 
> -    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getKeypairDelegate", PARAMS_CLASS,
> +    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getKeypairDelegate", "()Lchip/devicecontroller/KeypairDelegate;",
> 
> -    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getRootCertificate", PARAMS_CLASS,
> +    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getRootCertificate", "()[B",
>  
>  -    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getIntermediateCertificate", PARAMS_CLASS,
> +    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getIntermediateCertificate", "()[B",
> 
> -    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getOperationalCertificate", PARAMS_CLASS,
> +    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getOperationalCertificate", "()[B",
> 
> -    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getIpk", PARAMS_CLASS, &getIpk);
> +    err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getIpk", "()[B",
> +                                                        &getIpk);
> 
> 

@robertfarnum
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please submit the above code to resolve this issue. I have similar changes to resolve the FindMethod issue. The method signatures above (not class signature) are correct.

Please sign in to comment.