Missing commit message. Contributed by must go in commit message and not
in subject.
On Thu, Dec 06, 2012 at 04:54:40PM +0100, Mohamed wrote:
>
> Signed-off-by: Mohamed <mohamed.kallel@pivasoftware.com>
> Signed-off-by: Ahmed ZRIBI <ahmed.zribi@pivasoftware.com>
We are using tabs and not spaces. This must be changed in the entire patch.
Also, I don't see that you have updated help output.
> ---
> ext/openwrt/config/freecwmp | 6 ++++
> ext/openwrt/scripts/freecwmp.sh | 47
> ++++++++++++++++++++++++++++++++++
> ext/openwrt/scripts/functions/common | 4 +++
> 3 files changed, 57 insertions(+), 0 deletions(-)
>
> diff --git a/ext/openwrt/config/freecwmp b/ext/openwrt/config/freecwmp
> index ba6fbda..3b2b4c3 100644
> --- a/ext/openwrt/config/freecwmp
> +++ b/ext/openwrt/config/freecwmp
> @@ -29,19 +29,25 @@ config scripts
> # freecwmp specific functions
> list location /usr/share/freecwmp/functions/device_info
> list get_value_function get_device_info
> + list get_name_function get_device_info_name
> list set_value_function set_device_info
> list get_value_function get_device_info_generic
> + list get_name_function get_device_info_generic_name
> list set_value_function set_device_info_generic
> list location /usr/share/freecwmp/functions/lan_device
> list get_value_function get_lan_device
> + list get_name_function get_lan_device_name
> list set_value_function set_lan_device
> list location /usr/share/freecwmp/functions/management_server
> list get_value_function get_management_server
> + list get_name_function get_management_server_name
> list set_value_function set_management_server
> list get_value_function get_management_server_generic
> + list get_name_function get_management_server_generic_name
> list set_value_function set_management_server_generic
> list location /usr/share/freecwmp/functions/wan_device
> list get_value_function get_wan_device
> + list get_name_function get_wan_device_name
> list set_value_function set_wan_device
> list location /usr/share/freecwmp/functions/misc
> list get_value_function get_misc
> diff --git a/ext/openwrt/scripts/freecwmp.sh b/ext/openwrt/scripts/freecwmp.sh
> index f2bebc8..c5a0d53 100644
> --- a/ext/openwrt/scripts/freecwmp.sh
> +++ b/ext/openwrt/scripts/freecwmp.sh
> @@ -68,6 +68,10 @@ case "$1" in
> elif [ "$2" = "value" ]; then
> __arg1="$3"
> action="get_value"
> + elif [ "$2" = "name" ]; then
> + __arg1="$3"
> + __arg2=`echo $4| tr '[A-Z]' '[a-z]'`
Please explain what are you trying to do here. Why do we need __arg2?
> + action="get_name"
> elif [ "$2" = "all" ]; then
> __arg1="$3"
> action="get_all"
> @@ -107,6 +111,7 @@ handle_scripts() {
> config_get prefix "$section" "prefix"
> config_list_foreach "$section" 'location' load_script
> config_get get_value_functions "$section" "get_value_function"
> + config_get get_name_functions "$section" "get_name_function"
> config_get set_value_functions "$section" "set_value_function"
> }
>
> @@ -163,6 +168,48 @@ if [ "$action" = "get_value" -o "$action" = "get_all" ];
> then
> fi
> fi
>
> +if [ "$action" = "get_name" -o "$action" = "get_all" ]; then
> + no_fault="0"
> + freecwmp_check_fault "$__arg1"
> + fault_code="$?"
> + if [ "$fault_code" = "0" ]; then
> + if [ \( "$__arg2" != "0" \) -a \( "$__arg2" != "1" \) -a \(
> "$__arg2" != "true" \) -a \( "$__arg2" != "false" \) ]; then
> + fault_code="$FAULT_CPE_INVALID_ARGUMENTS"
> + else
> + if [ "$__arg2" = "true" ]; then
> + __arg2="1"
> + elif [ "$__arg2" = "false" ]; then
> + __arg2="0"
> + fi
I don't like that you are changing here __arxX variables. This needs to be
reworked.
> + fi
> + if [ "$fault_code" = "0" ]; then
> + if [ \( "$__arg1" = "InternetGatewayDevice." \) -o \( "$__arg1"
> = "" \) ]; then
Do we really need "\(" ?
> + ubus_freecwmp_output "InternetGatewayDevice." "0"
> + if [ \( "$__arg1" = "" \) -a \( "$__arg2" = "1" \) ]; then
> + exit 0
> + fi
> + __arg1="InternetGatewayDevice."
> + fi
> + for function_name in $get_name_functions
> + do
> + $function_name "$__arg1" "$__arg2"
> + fault_code="$?"
> + if [ "$fault_code" = "0" ]; then
> + no_fault="1"
> + fi
> + if [ "$fault_code" = "$FAULT_CPE_INVALID_ARGUMENTS" ]; then
> + break
> + fi
> + done
> + if [ "$no_fault" = "1" ]; then fault_code="0"; fi
Use multiple lines here. Also, no_fault error checking can be optimized here.
> + fi
> + fi
> + if [ "$fault_code" != "0" ]; then
> + let fault_code=$fault_code+9000
Please explain the logic behind this. We should use 9000 when defining the
variables.
> + ubus_freecwmp_output "$__arg1" "" "" "$fault_code"
> + fi
> +fi
> +
> if [ "$action" = "set_value" ]; then
> for function_name in $set_value_functions
> do
> diff --git a/ext/openwrt/scripts/functions/common
> b/ext/openwrt/scripts/functions/common
> index 082e067..abba220 100644
> --- a/ext/openwrt/scripts/functions/common
> +++ b/ext/openwrt/scripts/functions/common
> @@ -48,6 +48,10 @@ case "$action" in
> get_value)
> ubus call tr069 get_parameter_values '{"parameter": "'$parameter'",
> "value": "'$value'", "type": "'$type'", "fault_code":"'$fault_code'"}' 2>
> /dev/null
> ;;
> + get_name)
> + ubus call tr069 get_parameter_names '{"parameter": "'$parameter'",
> "writable": "'$value'", "fault_code":"'$fault_code'"}' 2> /dev/null
> + ;;
It's not good idea to use the same $value variable for two use cases.
This needs to be changed. Look at my comments in the next patch.
> +
Remove this newline.
> esac
> }
> freecwmp_value_output() {
> --
> 1.7.4.1
Luka
|