freecwmp
[Top] [All Lists]

Re: [PATCH 00/27] Data model with ubus

To: mohamed.kallel@pivasoftware.com
Subject: Re: [PATCH 00/27] Data model with ubus
From: Luka Perkov <freecwmp@lukaperkov.net>
Date: Sat, 8 Dec 2012 10:17:06 +0100
Cc: freecwmp@linux-mips.org, freecwmp@lukaperkov.net, jogo@openwrt.org
In-reply-to: <1354809292-2467-1-git-send-email-mohamed.kallel@pivasoftware.com>
Mail-followup-to: mohamed.kallel@pivasoftware.com, freecwmp@linux-mips.org, 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>
Sender: freecwmp-bounce@linux-mips.org
User-agent: Mutt/1.5.21 (2010-09-15)
Hi Mohamed,

On Thu, Dec 06, 2012 at 04:54:25PM +0100, Mohamed wrote:
> Hi Luka

Please next time put also jogo@openwrt.org in CC.

> As summary, The following patches contains:
> 
>   communication from script to the core via ubus
>   GetParameterValues method supporting many parameters
>   GetParameterNames method added

This is my general comment on your patch series:

* all patches are missing commit messages
* commit messages should be verbose
* commit subject should be shorter and it can not contain "Contributed
  by Inteno Broadband"
* "Contributed by Inteno Broadband" must go in commit message
* don't make coding style errors on one patch and then fix it later in
  the series
* please subscribe to the list so your patches don't get picked up by
  spam filter (that is why I did not deleted anything from my responses)

When sending updated patch make sure you name it PATCHv2 in the subject
instead PATCH only. Also, note the changes that you have made from the
last version. You can use u-boot patch submission guidelines as a
reference:

http://www.denx.de/wiki/U-Boot/Patches

You can look at this patch as an example:

http://lists.denx.de/pipermail/u-boot/2012-July/128709.html

But in your case commit messages must be much more verbose.

Regards,
Luka

> Regards
> Mohamed KALLEL
> 
> Mohamed (27):
>   add ubus listner get_parameter_values. This listener allows to get
>     the freecwmp output from extenal command.       Contributed by
>     Inteno Broadband Technology AB
>   get_parameter values handler from xml is supporting many parameters
>     from external ubus output       Contributed by Inteno Broadband
>     Technology AB && PIVA SOFTWARE
>   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
>   Update device_hosts to support communicte data of get parameter
>     values to the core via ubus       Contributed by Inteno Broadband
>     Technology AB
>   Update device_info script to support communicte data of get parameter
>     values to the core via ubus       Contributed by Inteno Broadband
>     Technology AB
>   Update device_routing script to support communicte data of get
>     parameter values to the core via ubus       Contributed by Inteno
>     Broadband Technology AB
>   Update device_users script to support communicte data of get
>     parameter values to the core via ubus       Contributed by Inteno
>     Broadband Technology AB
>   Update lan_device script to support communicte data of get parameter
>     values to the core via ubus       Contributed by Inteno Broadband
>     Technology AB
>   Update management_server script to support communicte data of get
>     parameter values to the core via ubus       Contributed by Inteno
>     Broadband Technology AB
>   Update misc script to support communicte data of get parameter values
>     to the core via ubus       Contributed by Inteno Broadband
>     Technology AB
>   Update wan_device script to support communicte data of get parameter
>     values to the core via ubus       Contributed by Inteno Broadband
>     Technology AB
>   update inform in order to get parameters from ubus instead from pipe 
>          Contributed by PIVA SOFTWARE
>   Add 3rd argument in the ubus handler of notification. The 3rd
>     argument is the type of the parameter      This agument could be 
>     used in the response of get parameter attribute       Contributed
>     by Inteno Broadband Technology AB
>   add xml message get_parameter_name handler.       Contributed by
>     Inteno Broadband Technology AB & PIVA SOFTWARE
>   add the common function and adapt the script freecwmp.sh in order to
>     support get parameter names       Contributed by Inteno Broadband
>     Technology AB
>   update the script device_info in order to output parameters with ubus
>           Contributed by Inteno Broadband Technology AB
>   update the script lan_device in order to output parameters with ubus 
>          Contributed by Inteno Broadband Technology AB
>   update the script management_server in order to output parameters
>     with ubus       Contributed by Inteno Broadband Technology AB
>   update the script wan_device in order to output parameters with ubus 
>          Contributed by Inteno Broadband Technology AB
>   update the get notification in order to put the returned data via
>     ubus instead of pipe      TODO: develop the xml handle message to
>     perform send of GetParameterAttributeResponse       Contributed by
>     Inteno Broadband Technology AB
>   update the data model parameter scripts in order to output parameter
>     and notification values via ubus       Contributed by Inteno
>     Broadband Technology AB
>   update xml handler set parameter values in order to execute external
>     command and get response via ubus       Contributed by Inteno
>     Broadband Technology AB
>   update common script and freecwmp.sh script in order to have the
>     functions for set parameter values working with ubus      
>     Contributed by Inteno Broadband Technology AB & PIVA SOFTWARE
>   update dm script to send the output of set parameter function to the
>     ubus       Contributed by Inteno Broadband Technology AB
>   add ubus listner for set parameter attribute       Contributed by
>     Inteno Broadband Technology AB & PIVA SOFTWARE
>   update common script and freecwmp.sh for set parameter attributes in
>     order to have the communication with the core via ubus      
>     Contributed by Inteno Broadband Technology AB
>   update the dm script function to communicate the output of set
>     parameter attribute with ubus       Contributed by Inteno Broadband
>     Technology AB
> 
>  bin/Makefile.am                                 |    3 +-
>  configure.ac                                    |    3 +
>  ext/openwrt/config/freecwmp                     |   18 +
>  ext/openwrt/scripts/freecwmp.sh                 |  219 ++++++-
>  ext/openwrt/scripts/functions/common            |   69 ++
>  ext/openwrt/scripts/functions/device_hosts      |   22 +-
>  ext/openwrt/scripts/functions/device_info       |  448 ++++++++++++-
>  ext/openwrt/scripts/functions/device_routing    |   33 +-
>  ext/openwrt/scripts/functions/device_users      |   13 +-
>  ext/openwrt/scripts/functions/lan_device        |  232 ++++++-
>  ext/openwrt/scripts/functions/management_server |  808 
> ++++++++++++++++++++---
>  ext/openwrt/scripts/functions/misc              |    8 +-
>  ext/openwrt/scripts/functions/wan_device        |  427 +++++++++++-
>  src/cwmp.c                                      |    4 +-
>  src/cwmp.h                                      |    2 +-
>  src/external.c                                  |  218 +++++--
>  src/external.h                                  |   26 +-
>  src/freecwmp.c                                  |   17 +
>  src/ubus.c                                      |  233 +++++++-
>  src/xml.c                                       |  260 ++++++--
>  src/xml.h                                       |    4 +
>  21 files changed, 2741 insertions(+), 326 deletions(-)
> 
> -- 
> 1.7.4.1
> 

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