Skip to content

Commit

Permalink
[java-matter-controller]Improve the readability and maintainability f…
Browse files Browse the repository at this point in the history
…rom code review (#29571)
  • Loading branch information
yufengwangca authored and pull[bot] committed Jan 25, 2024
1 parent d22e9cb commit 1206438
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,9 @@ import java.util.concurrent.atomic.AtomicLong
* that may be performed. Commands are verb-like, such as pair a Matter device or discover Matter
* devices from the environment.
*/
abstract class Command(private val name: String, private val helpText: String? = null) {
abstract class Command(val name: String, val helpText: String? = null) {
private val arguments = ArrayList<Argument>()

fun getName(): String {
return name
}

fun getHelpText(): String? {
return helpText
}

fun getArgumentName(index: Int): String {
return arguments[index].name
}
Expand Down Expand Up @@ -92,8 +84,8 @@ abstract class Command(private val name: String, private val helpText: String? =
* @return The number of arguments currently added to the command
* @brief Add a bool command argument
*/
fun addArgument(name: String?, out: AtomicBoolean?, desc: String?, optional: Boolean) {
val arg = Argument(name!!, out!!, desc, optional)
fun addArgument(name: String, out: AtomicBoolean, desc: String?, optional: Boolean) {
val arg = Argument(name, out, desc, optional)
addArgumentToList(arg)
}

Expand All @@ -108,14 +100,14 @@ abstract class Command(private val name: String, private val helpText: String? =
* @brief Add a short command argument
*/
fun addArgument(
name: String?,
name: String,
min: Short,
max: Short,
out: AtomicInteger?,
out: AtomicInteger,
desc: String?,
optional: Boolean
) {
val arg = Argument(name!!, min, max, out!!, desc, optional)
val arg = Argument(name, min, max, out, desc, optional)
addArgumentToList(arg)
}

Expand All @@ -130,14 +122,14 @@ abstract class Command(private val name: String, private val helpText: String? =
* @brief Add an int command argument
*/
fun addArgument(
name: String?,
name: String,
min: Int,
max: Int,
out: AtomicInteger?,
out: AtomicInteger,
desc: String?,
optional: Boolean
) {
val arg = Argument(name!!, min, max, out!!, desc, optional)
val arg = Argument(name, min, max, out, desc, optional)
addArgumentToList(arg)
}

Expand All @@ -152,14 +144,14 @@ abstract class Command(private val name: String, private val helpText: String? =
* @brief Add a long Integer command argument
*/
fun addArgument(
name: String?,
name: String,
min: Short,
max: Short,
out: AtomicLong?,
out: AtomicLong,
desc: String?,
optional: Boolean
) {
val arg = Argument(name!!, min, max, out!!, desc, optional)
val arg = Argument(name, min, max, out, desc, optional)
addArgumentToList(arg)
}

Expand All @@ -174,14 +166,14 @@ abstract class Command(private val name: String, private val helpText: String? =
* @brief Add a long Integer command argument
*/
fun addArgument(
name: String?,
name: String,
min: Long,
max: Long,
out: AtomicLong?,
out: AtomicLong,
desc: String?,
optional: Boolean
) {
val arg = Argument(name!!, min, max, out!!, desc, optional)
val arg = Argument(name, min, max, out, desc, optional)
addArgumentToList(arg)
}

Expand All @@ -192,8 +184,8 @@ abstract class Command(private val name: String, private val helpText: String? =
* @return The number of arguments currently added to the command
* @brief Add an IP address command argument
*/
fun addArgument(name: String?, out: IPAddress?, optional: Boolean) {
val arg = Argument(name!!, out!!, optional)
fun addArgument(name: String, out: IPAddress, optional: Boolean) {
val arg = Argument(name, out, optional)
addArgumentToList(arg)
}

Expand All @@ -205,16 +197,16 @@ abstract class Command(private val name: String, private val helpText: String? =
* @return The number of arguments currently added to the command
* @brief Add a String command argument
*/
fun addArgument(name: String?, out: StringBuffer?, desc: String?, optional: Boolean) {
val arg = Argument(name!!, out!!, desc, optional)
fun addArgument(name: String, out: StringBuffer, desc: String?, optional: Boolean) {
val arg = Argument(name, out, desc, optional)
addArgumentToList(arg)
}

/**
* @param args Supplied command-line arguments as an array of String objects.
* @brief Initialize command arguments
*/
fun initArguments(args: Array<String>) {
fun setArgumentValues(args: Array<String>) {
val argc = args.size
var mandatoryArgsCount = 0
var currentIndex = 0
Expand All @@ -224,12 +216,12 @@ abstract class Command(private val name: String, private val helpText: String? =
}
}
require(argc >= mandatoryArgsCount) {
"initArguments: Wrong arguments number: $argc instead of $mandatoryArgsCount"
"setArgumentValues: Wrong arguments number: $argc instead of $mandatoryArgsCount"
}

// Initialize mandatory arguments
for (i in 0 until mandatoryArgsCount) {
initArgument(currentIndex++, args[i])
setArgumentValue(currentIndex++, args[i])
}

// Initialize optional arguments
Expand All @@ -242,18 +234,20 @@ abstract class Command(private val name: String, private val helpText: String? =
!(args[i].length <= OPTIONAL_ARGUMENT_PREFIX_LENGTH &&
!args[i].startsWith(OPTIONAL_ARGUMENT_PREFIX))
) {
"initArguments: Invalid optional argument: " + args[i]
"setArgumentValues: Invalid optional argument: " + args[i]
}
if (args[i].substring(OPTIONAL_ARGUMENT_PREFIX_LENGTH) == arguments[currentIndex].name) {
require(i + 1 < argc) { "initArguments: Optional argument " + args[i] + " missing value" }
initArgument(currentIndex++, args[i + 1])
require(i + 1 < argc) {
"setArgumentValues: Optional argument " + args[i] + " missing value"
}
setArgumentValue(currentIndex++, args[i + 1])
}
i += 2
}
}

fun initArgument(argIndex: Int, argValue: String?) {
arguments[argIndex].setValue(argValue!!)
private fun setArgumentValue(argIndex: Int, argValue: String) {
arguments[argIndex].setValue(argValue)
}

@Throws(Exception::class) abstract fun run()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ class CommandManager {
val command: Command?
if (args.size < 1) {
logger.log(Level.INFO, "Missing cluster name")
showClusters()
showHelpInfo()
return
}
val commands = clusters[args[0]]
if (commands == null) {
logger.log(Level.INFO, "Unknown cluster: " + args[0])
showClusters()
showHelpInfo()
return
}
if (args.size < 2) {
Expand Down Expand Up @@ -85,7 +85,7 @@ class CommandManager {
// need skip over binary and command name and only get arguments
val temp = Arrays.copyOfRange(args, 2, args.size)
try {
command.initArguments(temp)
command.setArgumentValues(temp)
} catch (e: IllegalArgumentException) {
logger.log(Level.INFO, "Arguments init failed with exception: " + e.message)
showCommand(args[0], command)
Expand All @@ -108,7 +108,7 @@ class CommandManager {

private fun getCommand(commands: List<Command>, commandName: String): Command? {
for (command in commands) {
if (commandName == command.getName()) {
if (commandName == command.name) {
return command
}
}
Expand All @@ -121,14 +121,14 @@ class CommandManager {
attributeName: String
): Command? {
for (command in commands) {
if (commandName == command.getName() && attributeName == command.getAttribute()) {
if (commandName == command.name && attributeName == command.getAttribute()) {
return command
}
}
return null
}

private fun showClusters() {
private fun showHelpInfo() {
logger.log(Level.INFO, "Usage:")
logger.log(Level.INFO, " java-matter-controller cluster_name command_name [param1 param2 ...]")
logger.log(Level.INFO, "\n")
Expand Down Expand Up @@ -176,7 +176,7 @@ class CommandManager {
var subscribeEventCommand = false
for (command in commands) {
var shouldPrint = true
val cmdName = command.getName()
val cmdName = command.name
if (isGlobalCommand(cmdName)) {
if (cmdName == "read" && !readCommand) {
readCommand = true
Expand Down Expand Up @@ -227,7 +227,7 @@ class CommandManager {
" +-------------------------------------------------------------------------------------+"
)
for (command in commands) {
if (commandName == command.getName()) {
if (commandName == command.name) {
System.out.printf(" | * %-82s|\n", command.getAttribute())
}
}
Expand Down Expand Up @@ -258,7 +258,7 @@ class CommandManager {
" +-------------------------------------------------------------------------------------+"
)
for (command in commands) {
if (commandName == command.getName()) {
if (commandName == command.name) {
System.out.printf(" | * %-82s|\n", command.getAttribute())
}
}
Expand All @@ -270,7 +270,7 @@ class CommandManager {

private fun showCommand(clusterName: String, command: Command) {
logger.log(Level.INFO, "Usage:")
var arguments: String? = command.getName()
var arguments: String? = command.name
var description = ""
val argumentsCount = command.getArgumentsCount()
for (i in 0 until argumentsCount) {
Expand All @@ -295,7 +295,7 @@ class CommandManager {
}
}
System.out.format(" java-matter-controller %s %s\n", clusterName, arguments)
val helpText = command.getHelpText()
val helpText = command.helpText
if (helpText != null) {
System.out.format("\n%s\n", helpText)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
package com.matter.controller.commands.common

/**
* @brief Credentials Issuer for the Command
* @details Contains all credential information of the issuer of the command, such as operational
* credentials for a given fabric, the DAC verifier of the commisioner, etc ..
* Credentials Issuer which contains all credential information of the issuer of the command, such
* as operational credentials for a given fabric, the DAC verifier of the commisioner, etc ..
*/
class CredentialsIssuer
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package com.matter.controller.commands.common

import java.util.concurrent.TimeoutException
import java.util.logging.Level
import java.util.logging.Logger

Expand All @@ -27,8 +28,6 @@ import java.util.logging.Logger
* duration elapsed without receiving the expected realResult, the runtime exception would be
* raised.
*/
class RealResultException(message: String) : RuntimeException(message)

class FutureResult {
private var realResult: RealResult? = null
private var timeoutMs: Long = 0
Expand All @@ -42,7 +41,7 @@ class FutureResult {
fun setRealResult(realResult: RealResult) {
synchronized(lock) {
if (this.realResult != null) {
throw RealResultException("Error, real result has been set!")
throw TimeoutException("Error, real result has been set!")
}
this.realResult = realResult
lock.notifyAll()
Expand All @@ -55,16 +54,16 @@ class FutureResult {
while (realResult == null) {
try {
if (System.currentTimeMillis() > start + timeoutMs) {
throw RealResultException("Timeout!")
throw TimeoutException("Timeout!")
}
lock.wait()
} catch (e: InterruptedException) {
logger.log(Level.INFO, "Wait Result failed with exception: " + e.message)
}
}
if (realResult?.getResult() == false) {
logger.log(Level.INFO, "Error: ${realResult?.getError()}")
throw RealResultException("Received failure test result")
if (realResult?.result == false) {
logger.log(Level.INFO, "Error: ${realResult?.error}")
throw TimeoutException("Received failure test result")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package com.matter.controller.commands.common
import chip.devicecontroller.ChipDeviceController
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicLong
import java.util.logging.Logger

abstract class MatterCommand(
private val chipDeviceController: ChipDeviceController,
Expand Down Expand Up @@ -111,8 +110,4 @@ abstract class MatterCommand(
fun clear() {
futureResult.clear()
}

companion object {
private val logger = Logger.getLogger(MatterCommand::class.java.name)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ package com.matter.controller.commands.common
* contain either a `true` value for `Success` or a `false` value in which case the failure will
* also have an error string explaining the reason of the failure associated with it.
*/
class RealResult(private val result: Boolean, private val error: String?) {
class RealResult(val result: Boolean, val error: String?) {
constructor() : this(true, null)

constructor(error: String?) : this(false, error)
Expand All @@ -39,12 +39,4 @@ class RealResult(private val result: Boolean, private val error: String?) {
return RealResult(error)
}
}

fun getResult(): Boolean {
return result
}

fun getError(): String? {
return error
}
}

0 comments on commit 1206438

Please sign in to comment.