Fix Matter command argument list mismatch error (#23721)
* Fix Matter command argument list mismatch error
* Address the review comments
diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Argument.java b/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Argument.java
index 559c244..c9d911b 100644
--- a/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Argument.java
+++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Argument.java
@@ -33,59 +33,78 @@
private final long mMax;
private final Object mValue;
private final Optional<String> mDesc;
+ private final boolean mOptional;
- public Argument(String name, IPAddress value) {
+ boolean isOptional() {
+ return mOptional;
+ }
+
+ public Argument(String name, IPAddress value, boolean optional) {
this.mName = name;
this.mType = ArgumentType.ADDRESS;
this.mMin = 0;
this.mMax = 0;
this.mValue = value;
this.mDesc = Optional.empty();
+ this.mOptional = optional;
}
- public Argument(String name, StringBuffer value, @Nullable String desc) {
+ public Argument(String name, StringBuffer value, @Nullable String desc, boolean optional) {
this.mName = name;
this.mType = ArgumentType.STRING;
this.mMin = 0;
this.mMax = 0;
this.mValue = value;
this.mDesc = Optional.ofNullable(desc);
+ this.mOptional = optional;
}
- public Argument(String name, AtomicBoolean value, @Nullable String desc) {
+ public Argument(String name, AtomicBoolean value, @Nullable String desc, boolean optional) {
this.mName = name;
this.mType = ArgumentType.BOOL;
this.mMin = 0;
this.mMax = 0;
this.mValue = value;
this.mDesc = Optional.ofNullable(desc);
+ this.mOptional = optional;
}
- public Argument(String name, short min, short max, AtomicInteger value, @Nullable String desc) {
+ public Argument(
+ String name,
+ short min,
+ short max,
+ AtomicInteger value,
+ @Nullable String desc,
+ boolean optional) {
this.mName = name;
this.mType = ArgumentType.NUMBER_INT16;
this.mMin = min;
this.mMax = max;
this.mValue = value;
this.mDesc = Optional.ofNullable(desc);
+ this.mOptional = optional;
}
- public Argument(String name, int min, int max, AtomicInteger value, @Nullable String desc) {
+ public Argument(
+ String name, int min, int max, AtomicInteger value, @Nullable String desc, boolean optional) {
this.mName = name;
this.mType = ArgumentType.NUMBER_INT32;
this.mMin = min;
this.mMax = max;
this.mValue = value;
this.mDesc = Optional.ofNullable(desc);
+ this.mOptional = optional;
}
- public Argument(String name, long min, long max, AtomicLong value, @Nullable String desc) {
+ public Argument(
+ String name, long min, long max, AtomicLong value, @Nullable String desc, boolean optional) {
this.mName = name;
this.mType = ArgumentType.NUMBER_INT64;
this.mMin = min;
this.mMax = max;
this.mValue = value;
this.mDesc = Optional.ofNullable(desc);
+ this.mOptional = optional;
}
public String getName() {
diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Command.java b/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Command.java
index 13ed473..16c3b30 100644
--- a/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Command.java
+++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/common/Command.java
@@ -32,6 +32,8 @@
* Matter devices from the environment.
*/
public abstract class Command {
+ private static final String OPTIONAL_ARGUMENT_PREFIX = "--";
+ private static final int OPTIONAL_ARGUMENT_PREFIX_LENGTH = 2;
private final String mName;
private final ArrayList<Argument> mArgs = new ArrayList<Argument>();
private final Optional<String> mHelpText;
@@ -82,17 +84,35 @@
return mArgs.get(index).getDesc();
}
+ public final void addArgumentToList(Argument arg, boolean optional) {
+ if (arg.isOptional() || mArgs.isEmpty()) {
+ // Safe to just append to the end of a list.
+ mArgs.add(arg);
+ return;
+ }
+
+ // mandatory arg needs to be inserted before the optional arguments.
+ int index = 0;
+ while (index < mArgs.size() && !mArgs.get(index).isOptional()) {
+ index++;
+ }
+
+ // Insert before the first optional arg.
+ mArgs.add(index, arg);
+ }
+
/**
* @brief Add a bool command argument
* @param name The name that will be displayed in the command help
* @param out A pointer to a MutableInteger where the argv value will be stored
* @param desc The description of the argument that will be displayed in the command help
+ * @param optional Indicate if an optional argument
* @return The number of arguments currently added to the command
*/
- public final int addArgument(String name, AtomicBoolean out, @Nullable String desc) {
- Argument arg = new Argument(name, out, desc);
- mArgs.add(arg);
- return mArgs.size();
+ public final void addArgument(
+ String name, AtomicBoolean out, @Nullable String desc, boolean optional) {
+ Argument arg = new Argument(name, out, desc, optional);
+ addArgumentToList(arg, optional);
}
/**
@@ -102,13 +122,18 @@
* @param max The minimum value of the argv value
* @param out A pointer to a MutableInteger where the argv value will be stored
* @param desc The description of the argument that will be displayed in the command help
+ * @param optional Indicate if an optional argument
* @return The number of arguments currently added to the command
*/
- public final int addArgument(
- String name, short min, short max, AtomicInteger out, @Nullable String desc) {
- Argument arg = new Argument(name, min, max, out, desc);
- mArgs.add(arg);
- return mArgs.size();
+ public final void addArgument(
+ String name,
+ short min,
+ short max,
+ AtomicInteger out,
+ @Nullable String desc,
+ boolean optional) {
+ Argument arg = new Argument(name, min, max, out, desc, optional);
+ addArgumentToList(arg, optional);
}
/**
@@ -118,13 +143,13 @@
* @param max The minimum value of the argv value
* @param out A pointer to a MutableInteger where the argv value will be stored
* @param desc The description of the argument that will be displayed in the command help
+ * @param optional Indicate if an optional argument
* @return The number of arguments currently added to the command
*/
- public final int addArgument(
- String name, int min, int max, AtomicInteger out, @Nullable String desc) {
- Argument arg = new Argument(name, min, max, out, desc);
- mArgs.add(arg);
- return mArgs.size();
+ public final void addArgument(
+ String name, int min, int max, AtomicInteger out, @Nullable String desc, boolean optional) {
+ Argument arg = new Argument(name, min, max, out, desc, optional);
+ addArgumentToList(arg, optional);
}
/**
@@ -134,25 +159,25 @@
* @param max The minimum value of the argv value
* @param out A pointer to a MutableInteger where the argv value will be stored
* @param desc The description of the argument that will be displayed in the command help
+ * @param optional Indicate if an optional argument
* @return The number of arguments currently added to the command
*/
- public final int addArgument(
- String name, long min, long max, AtomicLong out, @Nullable String desc) {
- Argument arg = new Argument(name, min, max, out, desc);
- mArgs.add(arg);
- return mArgs.size();
+ public final void addArgument(
+ String name, long min, long max, AtomicLong out, @Nullable String desc, boolean optional) {
+ Argument arg = new Argument(name, min, max, out, desc, optional);
+ addArgumentToList(arg, optional);
}
/**
* @brief Add an IP address command argument
* @param name The name that will be displayed in the command help
* @param out A pointer to a IPAddress where the argv value will be stored
+ * @param optional Indicate if an optional argument
* @return The number of arguments currently added to the command
*/
- public final int addArgument(String name, IPAddress out) {
- Argument arg = new Argument(name, out);
- mArgs.add(arg);
- return mArgs.size();
+ public final void addArgument(String name, IPAddress out, boolean optional) {
+ Argument arg = new Argument(name, out, optional);
+ addArgumentToList(arg, optional);
}
/**
@@ -160,12 +185,13 @@
* @param name The name that will be displayed in the command help
* @param out A pointer to a StringBuffer where the argv value will be stored
* @param desc The description of the argument that will be displayed in the command help
+ * @param optional Indicate if an optional argument
* @return The number of arguments currently added to the command
*/
- public final int addArgument(String name, StringBuffer out, @Nullable String desc) {
- Argument arg = new Argument(name, out, desc);
- mArgs.add(arg);
- return mArgs.size();
+ public final void addArgument(
+ String name, StringBuffer out, @Nullable String desc, boolean optional) {
+ Argument arg = new Argument(name, out, desc, optional);
+ addArgumentToList(arg, optional);
}
/**
@@ -174,15 +200,44 @@
* @param args Supplied command-line arguments as an array of String objects.
*/
public final void initArguments(int argc, String[] args) {
- int argsCount = mArgs.size();
+ int mandatoryArgsCount = 0;
+ int currentIndex = 0;
- if (argsCount != argc) {
- throw new IllegalArgumentException(
- "Wrong arguments number: " + argc + " instead of " + argsCount);
+ for (Argument arg : mArgs) {
+ if (!arg.isOptional()) {
+ mandatoryArgsCount++;
+ }
}
- for (int i = 0; i < argsCount; i++) {
- initArgument(i, args[i]);
+ if (argc < mandatoryArgsCount) {
+ throw new IllegalArgumentException(
+ "initArguments: Wrong arguments number: " + argc + " instead of " + mandatoryArgsCount);
+ }
+
+ // Initialize mandatory arguments
+ for (int i = 0; i < mandatoryArgsCount; i++) {
+ initArgument(currentIndex++, args[i]);
+ }
+
+ // Initialize optional arguments
+ // Optional arguments expect a name and a value, so i is increased by 2 on every step.
+ for (int i = mandatoryArgsCount; i < argc; i += 2) {
+ // optional arguments starts with OPTIONAL_ARGUMENT_PREFIX
+ if (args[i].length() <= OPTIONAL_ARGUMENT_PREFIX_LENGTH
+ && !args[i].startsWith(OPTIONAL_ARGUMENT_PREFIX)) {
+ throw new IllegalArgumentException("initArguments: Invalid optional argument: " + args[i]);
+ }
+
+ if (args[i]
+ .substring(OPTIONAL_ARGUMENT_PREFIX_LENGTH)
+ .equals(mArgs.get(currentIndex).getName())) {
+ if (i + 1 >= argc) {
+ throw new IllegalArgumentException(
+ "initArguments: Optional argument " + args[i] + " missing value");
+ }
+
+ initArgument(currentIndex++, args[i + 1]);
+ }
}
}
diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/common/MatterCommand.java b/examples/java-matter-controller/java/src/com/matter/controller/commands/common/MatterCommand.java
index 9aefec5..d78a7bb 100644
--- a/examples/java-matter-controller/java/src/com/matter/controller/commands/common/MatterCommand.java
+++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/common/MatterCommand.java
@@ -57,40 +57,43 @@
this.mCredIssuerCmds = Optional.ofNullable(credIssuerCmds);
this.mChipDeviceController = controller;
- // TODO: Add support to enable the below optional arguments
- /*
addArgument(
- "paa-trust-store-path",
+ "--paa-trust-store-path",
mPaaTrustStorePath,
"Path to directory holding PAA certificate information. Can be absolute or relative to the current working "
- + "directory.");
+ + "directory.",
+ true);
addArgument(
- "cd-trust-store-path",
+ "--cd-trust-store-path",
mCDTrustStorePath,
"Path to directory holding CD certificate information. Can be absolute or relative to the current working "
- + "directory.");
+ + "directory.",
+ true);
addArgument(
- "commissioner-name",
+ "--commissioner-name",
mCommissionerName,
"Name of fabric to use. Valid values are \"alpha\", \"beta\", \"gamma\", and integers greater than or equal to "
- + "4. The default if not specified is \"alpha\".");
+ + "4. The default if not specified is \"alpha\".",
+ true);
addArgument(
- "commissioner-nodeid",
+ "--commissioner-nodeid",
0,
Long.MAX_VALUE,
mCommissionerNodeId,
- "The node id to use for chip-tool. If not provided, kTestControllerNodeId (112233, 0x1B669) will be used.");
+ "The node id to use for chip-tool. If not provided, kTestControllerNodeId (112233, 0x1B669) will be used.",
+ true);
addArgument(
- "use-max-sized-certs",
+ "--use-max-sized-certs",
mUseMaxSizedCerts,
"Maximize the size of operational certificates. If not provided or 0 (\"false\"), normally sized operational "
- + "certificates are generated.");
+ + "certificates are generated.",
+ true);
addArgument(
- "only-allow-trusted-cd-keys",
+ "--only-allow-trusted-cd-keys",
mOnlyAllowTrustedCdKeys,
"Only allow trusted CD verifying keys (disallow test keys). If not provided or 0 (\"false\"), untrusted CD "
- + "verifying keys are allowed. If 1 (\"true\"), test keys are disallowed.");
- */
+ + "verifying keys are allowed. If 1 (\"true\"), test keys are disallowed.",
+ true);
}
// This method returns the commissioner instance to be used for running the command.
diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/discover/DiscoverCommand.java b/examples/java-matter-controller/java/src/com/matter/controller/commands/discover/DiscoverCommand.java
index 3d0bf25..988e97b 100644
--- a/examples/java-matter-controller/java/src/com/matter/controller/commands/discover/DiscoverCommand.java
+++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/discover/DiscoverCommand.java
@@ -29,8 +29,8 @@
public DiscoverCommand(ChipDeviceController controller, CredentialsIssuer credsIssuer) {
super(controller, "resolve", credsIssuer);
- addArgument("nodeid", 0, Long.MAX_VALUE, mNodeId, null);
- addArgument("fabricid", 0, Long.MAX_VALUE, mFabricId, null);
+ addArgument("nodeid", 0, Long.MAX_VALUE, mNodeId, null, false);
+ addArgument("fabricid", 0, Long.MAX_VALUE, mFabricId, null, false);
}
@Override
diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/CloseSessionCommand.java b/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/CloseSessionCommand.java
index ce3e263..f8b9112 100644
--- a/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/CloseSessionCommand.java
+++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/CloseSessionCommand.java
@@ -30,13 +30,14 @@
public CloseSessionCommand(ChipDeviceController controller, CredentialsIssuer credsIssuer) {
super(controller, "close-session", credsIssuer);
- addArgument("destination-id", 0, Long.MAX_VALUE, mDestinationId, null);
+ addArgument("destination-id", 0, Long.MAX_VALUE, mDestinationId, null, false);
addArgument(
"timeout",
(short) 0,
Short.MAX_VALUE,
mTimeoutSecs,
- "Time, in seconds, before this command is considered to have timed out.");
+ "Time, in seconds, before this command is considered to have timed out.",
+ false);
}
@Override
diff --git a/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairingCommand.java b/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairingCommand.java
index 323c3bd..2d4a6bb 100644
--- a/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairingCommand.java
+++ b/examples/java-matter-controller/java/src/com/matter/controller/commands/pairing/PairingCommand.java
@@ -163,18 +163,18 @@
throw new RuntimeException(e);
}
- addArgument("node-id", 0, Long.MAX_VALUE, mNodeId, null);
+ addArgument("node-id", 0, Long.MAX_VALUE, mNodeId, null, false);
switch (networkType) {
case NONE:
case ETHERNET:
break;
case WIFI:
- addArgument("ssid", mSSID, null);
- addArgument("password", mPassword, null);
+ addArgument("ssid", mSSID, null, false);
+ addArgument("password", mPassword, null, false);
break;
case THREAD:
- addArgument("operationalDataset", mOperationalDataset, null);
+ addArgument("operationalDataset", mOperationalDataset, null, false);
break;
}
@@ -184,29 +184,29 @@
case CODE:
case CODE_PASE_ONLY:
Only:
- addArgument("payload", mOnboardingPayload, null);
- addArgument("discover-once", mDiscoverOnce, null);
- addArgument("use-only-onnetwork-discovery", mUseOnlyOnNetworkDiscovery, null);
+ addArgument("payload", mOnboardingPayload, null, false);
+ addArgument("discover-once", mDiscoverOnce, null, false);
+ addArgument("use-only-onnetwork-discovery", mUseOnlyOnNetworkDiscovery, null, false);
break;
case BLE:
- addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null);
- addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null);
+ addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null, false);
+ addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null, false);
break;
case ON_NETWORK:
- addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null);
+ addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null, false);
break;
case SOFT_AP:
AP:
- addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null);
- addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null);
- addArgument("device-remote-ip", mRemoteAddr);
- addArgument("device-remote-port", (short) 0, Short.MAX_VALUE, mRemotePort, null);
+ addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null, false);
+ addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null, false);
+ addArgument("device-remote-ip", mRemoteAddr, false);
+ addArgument("device-remote-port", (short) 0, Short.MAX_VALUE, mRemotePort, null, false);
break;
case ETHERNET:
- addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null);
- addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null);
- addArgument("device-remote-ip", mRemoteAddr);
- addArgument("device-remote-port", (short) 0, Short.MAX_VALUE, mRemotePort, null);
+ addArgument("setup-pin-code", 0, 134217727, mSetupPINCode, null, false);
+ addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null, false);
+ addArgument("device-remote-ip", mRemoteAddr, false);
+ addArgument("device-remote-port", (short) 0, Short.MAX_VALUE, mRemotePort, null, false);
break;
}
@@ -214,28 +214,28 @@
case NONE:
break;
case SHORT_DISCRIMINATOR:
- addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null);
+ addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null, false);
break;
case LONG_DISCRIMINATOR:
- addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null);
+ addArgument("discriminator", (short) 0, (short) 4096, mDiscriminator, null, false);
break;
case VENDOR_ID:
- addArgument("vendor-id", (short) 0, Short.MAX_VALUE, mDiscoveryFilterCode, null);
+ addArgument("vendor-id", (short) 0, Short.MAX_VALUE, mDiscoveryFilterCode, null, false);
break;
case COMPRESSED_FABRIC_ID:
- addArgument("fabric-id", 0, Long.MAX_VALUE, mDiscoveryFilterCode, null);
+ addArgument("fabric-id", 0, Long.MAX_VALUE, mDiscoveryFilterCode, null, false);
break;
case COMMISSIONING_MODE:
case COMMISSIONER:
break;
case DEVICE_TYPE:
- addArgument("device-type", (short) 0, Short.MAX_VALUE, mDiscoveryFilterCode, null);
+ addArgument("device-type", (short) 0, Short.MAX_VALUE, mDiscoveryFilterCode, null, false);
break;
case INSTANCE_NAME:
- addArgument("name", mDiscoveryFilterInstanceName, null);
+ addArgument("name", mDiscoveryFilterInstanceName, null, false);
break;
}
- addArgument("timeout", (long) 0, Long.MAX_VALUE, mTimeoutMillis, null);
+ addArgument("timeout", (long) 0, Long.MAX_VALUE, mTimeoutMillis, null, false);
}
}
