Re: [OpenWrt-Devel] [PATCH 1/4] base-files: add option to specify netdev led mode in configuration generation

2016-01-19 Thread Jo-Philipp Wich 
Hi,

see inline comments.

~ Jow

On 01/07/2016 01:40 AM, Tal Keren wrote:
> This is necessary for controlling leds of RJ45 port, when one indicate the 
> link
> status and the other indicate data transfer.
> 
> Signed-off-by: Tal Keren 
> ---
>  package/base-files/files/bin/config_generate   | 7 ---
>  package/base-files/files/lib/functions/uci-defaults.sh | 2 ++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/package/base-files/files/bin/config_generate 
> b/package/base-files/files/bin/config_generate
> index 9218788..4f257e4 100755
> --- a/package/base-files/files/bin/config_generate
> +++ b/package/base-files/files/bin/config_generate
> @@ -257,11 +257,12 @@ generate_led() {
>   ;;
>  
>   netdev)
> - local device
> - json_get_vars device
> + local device mode
> + json_get_vars device mode
> + [ -n "$mode" ] || mode='link tx rx'

Remove this check/set.

>   uci -q batch <<-EOF
>   set system.$cfg.trigger='netdev'
> - set system.$cfg.mode='link tx rx'
> + set system.$cfg.mode='$mode'

Use "set system.$cfg.mode='${mode:-link tx rx}'" here.

>   set system.$cfg.dev='$device'
>   EOF
>   ;;
> diff --git a/package/base-files/files/lib/functions/uci-defaults.sh 
> b/package/base-files/files/lib/functions/uci-defaults.sh
> index de3f180..c0ff98a 100755
> --- a/package/base-files/files/lib/functions/uci-defaults.sh
> +++ b/package/base-files/files/lib/functions/uci-defaults.sh
> @@ -355,6 +355,7 @@ ucidef_set_led_netdev() {
>   local name="$2"
>   local sysfs="$3"
>   local dev="$4"
> + local mode="$5"
>  
>   json_select_object led
>  
> @@ -363,6 +364,7 @@ ucidef_set_led_netdev() {
>   json_add_string type netdev
>   json_add_string sysfs "$sysfs"
>   json_add_string device "$dev"
> + [ -n "$mode" ] && json_add_string mode "$mode"

Remove the [ -n ... ] test, empty values are ignored and do not result
in a set.

>   json_select ..
>  
>   json_select ..
> 
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH 1/4] base-files: add option to specify netdev led mode in configuration generation

2016-01-19 Thread John Crispin 


On 07/01/2016 01:40, Tal Keren wrote:
> This is necessary for controlling leds of RJ45 port, when one indicate the 
> link
> status and the other indicate data transfer.
> 
> Signed-off-by: Tal Keren 
> ---
>  package/base-files/files/bin/config_generate   | 7 ---
>  package/base-files/files/lib/functions/uci-defaults.sh | 2 ++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/package/base-files/files/bin/config_generate 
> b/package/base-files/files/bin/config_generate
> index 9218788..4f257e4 100755
> --- a/package/base-files/files/bin/config_generate
> +++ b/package/base-files/files/bin/config_generate
> @@ -257,11 +257,12 @@ generate_led() {
>   ;;
>  
>   netdev)
> - local device
> - json_get_vars device
> + local device mode
> + json_get_vars device mode
> + [ -n "$mode" ] || mode='link tx rx'

move this line to ucidef_set_led_netdev() and then always set the mode.
this will make the patch a lot simpler

John


>   uci -q batch <<-EOF
>   set system.$cfg.trigger='netdev'
> - set system.$cfg.mode='link tx rx'
> + set system.$cfg.mode='$mode'
>   set system.$cfg.dev='$device'
>   EOF
>   ;;
> diff --git a/package/base-files/files/lib/functions/uci-defaults.sh 
> b/package/base-files/files/lib/functions/uci-defaults.sh
> index de3f180..c0ff98a 100755
> --- a/package/base-files/files/lib/functions/uci-defaults.sh
> +++ b/package/base-files/files/lib/functions/uci-defaults.sh
> @@ -355,6 +355,7 @@ ucidef_set_led_netdev() {
>   local name="$2"
>   local sysfs="$3"
>   local dev="$4"
> + local mode="$5"
>  
>   json_select_object led
>  
> @@ -363,6 +364,7 @@ ucidef_set_led_netdev() {
>   json_add_string type netdev
>   json_add_string sysfs "$sysfs"
>   json_add_string device "$dev"
> + [ -n "$mode" ] && json_add_string mode "$mode"
>   json_select ..
>  
>   json_select ..
> 
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH 1/4] base-files: add option to specify netdev led mode in configuration generation

2016-01-19 Thread Jo-Philipp Wich 
Hi,

thought some more about it and we want to have the proper defaults in
board.json already, therefore disregard my previous comments and stick
to the ones below. Sorry for the bikeshedding here.

Thanks,
Jow

> On 01/07/2016 01:40 AM, Tal Keren wrote:
>> This is necessary for controlling leds of RJ45 port, when one indicate the 
>> link
>> status and the other indicate data transfer.
>>
>> Signed-off-by: Tal Keren 
>> ---
>>  package/base-files/files/bin/config_generate   | 7 ---
>>  package/base-files/files/lib/functions/uci-defaults.sh | 2 ++
>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/package/base-files/files/bin/config_generate 
>> b/package/base-files/files/bin/config_generate
>> index 9218788..4f257e4 100755
>> --- a/package/base-files/files/bin/config_generate
>> +++ b/package/base-files/files/bin/config_generate
>> @@ -257,11 +257,12 @@ generate_led() {
>>  ;;
>>  
>>  netdev)
>> -local device
>> -json_get_vars device
>> +local device mode
>> +json_get_vars device mode
>> +[ -n "$mode" ] || mode='link tx rx'

Still remove this check/set.

>>  uci -q batch <<-EOF
>>  set system.$cfg.trigger='netdev'
>> -set system.$cfg.mode='link tx rx'
>> +set system.$cfg.mode='$mode'

Keep as you proposed already.

>>  set system.$cfg.dev='$device'
>>  EOF
>>  ;;
>> diff --git a/package/base-files/files/lib/functions/uci-defaults.sh 
>> b/package/base-files/files/lib/functions/uci-defaults.sh
>> index de3f180..c0ff98a 100755
>> --- a/package/base-files/files/lib/functions/uci-defaults.sh
>> +++ b/package/base-files/files/lib/functions/uci-defaults.sh
>> @@ -355,6 +355,7 @@ ucidef_set_led_netdev() {
>>  local name="$2"
>>  local sysfs="$3"
>>  local dev="$4"
>> +local mode="$5"
>>  
>>  json_select_object led
>>  
>> @@ -363,6 +364,7 @@ ucidef_set_led_netdev() {
>>  json_add_string type netdev
>>  json_add_string sysfs "$sysfs"
>>  json_add_string device "$dev"
>> +[ -n "$mode" ] && json_add_string mode "$mode"

Still remove the [ -n ... ] test and replace the line with
  json_add_string mode "${mode:-link tx rx}"

> 
>>  json_select ..
>>  
>>  json_select ..
>>
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
> 
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

[OpenWrt-Devel] [PATCH 1/4] base-files: add option to specify netdev led mode in configuration generation

2016-01-06 Thread Tal Keren 
This is necessary for controlling leds of RJ45 port, when one indicate the link
status and the other indicate data transfer.

Signed-off-by: Tal Keren 
---
 package/base-files/files/bin/config_generate   | 7 ---
 package/base-files/files/lib/functions/uci-defaults.sh | 2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/package/base-files/files/bin/config_generate 
b/package/base-files/files/bin/config_generate
index 9218788..4f257e4 100755
--- a/package/base-files/files/bin/config_generate
+++ b/package/base-files/files/bin/config_generate
@@ -257,11 +257,12 @@ generate_led() {
;;
 
netdev)
-   local device
-   json_get_vars device
+   local device mode
+   json_get_vars device mode
+   [ -n "$mode" ] || mode='link tx rx'
uci -q batch <<-EOF
set system.$cfg.trigger='netdev'
-   set system.$cfg.mode='link tx rx'
+   set system.$cfg.mode='$mode'
set system.$cfg.dev='$device'
EOF
;;
diff --git a/package/base-files/files/lib/functions/uci-defaults.sh 
b/package/base-files/files/lib/functions/uci-defaults.sh
index de3f180..c0ff98a 100755
--- a/package/base-files/files/lib/functions/uci-defaults.sh
+++ b/package/base-files/files/lib/functions/uci-defaults.sh
@@ -355,6 +355,7 @@ ucidef_set_led_netdev() {
local name="$2"
local sysfs="$3"
local dev="$4"
+   local mode="$5"
 
json_select_object led
 
@@ -363,6 +364,7 @@ ucidef_set_led_netdev() {
json_add_string type netdev
json_add_string sysfs "$sysfs"
json_add_string device "$dev"
+   [ -n "$mode" ] && json_add_string mode "$mode"
json_select ..
 
json_select ..
-- 
2.6.4
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel