freecwmp
[Top] [All Lists]

Re: [PATCH 03/27] add in the scripts the functions needed to communicate

To: mohamed.kallel@pivasoftware.com
Subject: Re: [PATCH 03/27] add in the scripts the functions needed to communicate data to ubus handler for the get_parameter_values ubus handler Contributed by Inteno Broadband Technology AB
From: Luka Perkov <freecwmp@lukaperkov.net>
Date: Sat, 8 Dec 2012 11:32:40 +0100
Cc: freecwmp@linux-mips.org, ahmed.zribi@pivasoftware.com, freecwmp@lukaperkov.net, jogo@openwrt.org
In-reply-to: <1354809292-2467-4-git-send-email-mohamed.kallel@pivasoftware.com>
Mail-followup-to: mohamed.kallel@pivasoftware.com, freecwmp@linux-mips.org, ahmed.zribi@pivasoftware.com, jogo@openwrt.org
Original-recipient: rfc822;freecwmp@linux-mips.org
References: <mohamed.kallel@pivasoftware.com> <1354809292-2467-1-git-send-email-mohamed.kallel@pivasoftware.com> <1354809292-2467-4-git-send-email-mohamed.kallel@pivasoftware.com>
Sender: freecwmp-bounce@linux-mips.org
User-agent: Mutt/1.5.21 (2010-09-15)
Missing commit message. Contributed by must go in commit message and not
in subject.

On Thu, Dec 06, 2012 at 04:54:28PM +0100, Mohamed wrote:
> 
> Signed-off-by: Ahmed ZRIBI <ahmed.zribi@pivasoftware.com>
> Signed-off-by: Mohamed <mohamed.kallel@pivasoftware.com>
> ---
>  ext/openwrt/scripts/freecwmp.sh      |   51 
> ++++++++++++++++++++++++++++------
>  ext/openwrt/scripts/functions/common |   23 +++++++++++++++
>  2 files changed, 65 insertions(+), 9 deletions(-)
> 
> diff --git a/ext/openwrt/scripts/freecwmp.sh b/ext/openwrt/scripts/freecwmp.sh
> index ccab19f..f2bebc8 100644
> --- a/ext/openwrt/scripts/freecwmp.sh
> +++ b/ext/openwrt/scripts/freecwmp.sh
> @@ -113,21 +113,54 @@ handle_scripts() {
>  config_load freecwmp
>  config_foreach handle_scripts "scripts"
>  
> +# Fault code

Rename to "Fault codes".

> +
> +FAULT_CPE_NO_FAULT="0"

We don't need this one. Use 0 when you need it.

> +FAULT_CPE_REQUEST_DENIED="1"
> +FAULT_CPE_INTERNAL_ERROR="2"
> +FAULT_CPE_INVALID_ARGUMENTS="3"
> +FAULT_CPE_RESOURCES_EXCEEDED="4"
> +FAULT_CPE_INVALID_PARAMETER_NAME="5"
> +FAULT_CPE_INVALID_PARAMETER_TYPE="6"
> +FAULT_CPE_INVALID_PARAMETER_VALUE="7"
> +FAULT_CPE_NON_WRITABLE_PARAMETER="8"
> +FAULT_CPE_NOTIFICATION_REJECTED="9"
> +FAULT_CPE_DOWNLOAD_FAILURE="10"
> +FAULT_CPE_UPLOAD_FAILURE="11"
> +FAULT_CPE_FILE_TRANSFER_AUTHENTICATION_FAILURE="12"
> +FAULT_CPE_FILE_TRANSFER_UNSUPPORTED_PROTOCOL="13"
> +FAULT_CPE_DOWNLOAD_FAIL_MULTICAST_GROUP="14"
> +FAULT_CPE_DOWNLOAD_FAIL_CONTACT_SERVER="15"
> +FAULT_CPE_DOWNLOAD_FAIL_ACCESS_FILE="16"
> +FAULT_CPE_DOWNLOAD_FAIL_COMPLETE_DOWNLOAD="17"
> +FAULT_CPE_DOWNLOAD_FAIL_FILE_CORRUPTED="18"
> +FAULT_CPE_DOWNLOAD_FAIL_FILE_AUTHENTICATION="19"

Rename it from "FAULT_CPE_" to "E_". Also, why didn't you use error codes from
the standard?

Error codes should also be added in libfreecwmp. I saw that you are using them
in freecwmp core too.

> +
>  if [ "$action" = "get_value" -o "$action" = "get_all" ]; then
> -     if [ ${FLAGS_force} -eq ${FLAGS_FALSE} ]; then
> -             __tmp_arg="Device."
> -             # TODO: don't check only string length ; but this is only used
> -             #       for getting correct prefix of CWMP parameter anyway
> -             if [  ${#__arg1} -lt ${#__tmp_arg} ]; then
> -                     echo "CWMP parameters usualy begin with 
> 'InternetGatewayDevice.' or 'Device.'     "
> -                     echo "if you want to force script execution with 
> provided parameter use '-f' flag."
> -                     exit -1
> -             fi

NACK. You don't need to remove this.

> +    no_fault="0"
> +    freecwmp_check_fault "$__arg1"
> +    fault_code="$?"
> +    if [ "$fault_code" = "0" ]; then
> +        if [ \( "$__arg1" = "InternetGatewayDevice." \) -o \( "$__arg1" = "" 
> \) ]; then
> +            __arg1="InternetGatewayDevice."
>       fi
>       for function_name in $get_value_functions
>       do
>               $function_name "$__arg1"
> +                             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
> +    fi
> +    if [ "$fault_code" != "0" ]; then
> +        let fault_code=$fault_code+9000
> +        ubus_freecwmp_output "$__arg1" "" "" "$fault_code"
> +    fi
>  fi
>  
>  if [ "$action" = "set_value" ]; then
> diff --git a/ext/openwrt/scripts/functions/common 
> b/ext/openwrt/scripts/functions/common
> index 7e3000a..082e067 100644
> --- a/ext/openwrt/scripts/functions/common
> +++ b/ext/openwrt/scripts/functions/common
> @@ -34,6 +34,22 @@ if [ -n "$value" -o ${FLAGS_empty} -eq ${FLAGS_TRUE} ]; 
> then
>  fi
>  }
>  
> +ubus_freecwmp_output() {

Rename to freecwmp_ubus_output. Also, this needs to be integrated with
freecwmp_output function. More about that in other comments.

> +local parameter="$1"
> +local value="$2"
> +local type="$3"
> +local fault_code="$4"
> +
> +if [ "$type" = "" ]; then
> +     type="xsd:string"
> +fi
> +
> +case "$action" in
> +     get_value)
> +     ubus call tr069 get_parameter_values '{"parameter": "'$parameter'", 
> "value": "'$value'", "type": "'$type'", "fault_code":"'$fault_code'"}' 2> 
> /dev/null

This needs to be used like the uci command:

/sbin/uci ${UCI_CONFIG_DIR:+-c $UCI_CONFIG_DIR}

You need to pass optional parameter to ubus command where the socket is
located.

> +     ;;
> +esac
> +}

Missing newline.

>  freecwmp_value_output() {
>       freecwmp_output "$1" "$2" "V"
>  }
> @@ -249,3 +265,10 @@ EOF
>       sh "$lock" &
>  fi
>  }
> +
> +freecwmp_check_fault() {
> +if [ "$1" = "." ]; then
> +     return $FAULT_CPE_INVALID_PARAMETER_NAME
> +fi
> +return $FAULT_CPE_NO_FAULT
> +}
> -- 
> 1.7.4.1

Luka 

<Prev in Thread] Current Thread [Next in Thread>