mkresin:

First of all, it isn't a bug it's intentionally.

And things a complicated with the HH5a. The hub has only three LEDs, but these LEDs are RGB LEDs, where each LED is controlled by three GPIOs. That is why you have red:broadband, green:broadband, blue:broadband.

The GPIO LED driver does not have support for RGB LEDs and you can not use these colours intendant. Means, if you enable red + blue, the LED will be purple. With red + blue + green the LED will be white. If you use some kind of blinking pattern you get nice colour effects. What is missing in the kernel driver is the possibility to define an LED with multiple GPIOs and the value of each GPIO.

I've implemented some kind of [[https://git.lede-project.org/?p=source.git;a=commitdiff;h=551c9c830027b7749895e3ba6b37004570b89e74;hp=32012decc327486bdf6564e6cf55a4acfc744efe|workaround for the power LED]] in failsafe, which disables the green power LED before enabling the red power LED.

I've decided back in the OpenWrt days(!), to show the Internet connection (ppp) status via the broadband LED. The broadband LED will shine blue if a ppp connection is established (link) and will blink blue on rx/tx.

But even having full support for RGB LEDs wouldn't allow to use a single LED for DSL and Internet connection status. It is not possible to use multiple trigger for a single LED.

Assuming we are using broadband:orange (red + green?) for the DSL line status (using red if everything is fine seams to me a bad idea). If the internet connection is terminated but the DSL line is still in sync, you would expect that the broadband LED switches back from blue to orange. But it isn't possible, since the link/rx/tx ppp trigger switches the LED off.

I'm not saying it isn't possible, but to my knowledge it requires a lot of hacks and workarounds to get such a config working. And at least to me it's questionable if the use case is worse adding these workarounds/hacks.

If you still interested in showing the DSL line status via the broadband LED, remove the "config led 'led_internet'" from your /etc/config/system and add the following instead:

ezplanet:

It would be nice to have it back as it was with OpenWrt. It was just fine.
That is: ppp is down, "b" blue goes out and turns red thanks to my trigger:

/etc/hotplug.d/05-wan_led

#!/bin/sh
BROADBAND_RED="/sys/class/leds/bthomehubv5a:red:broadband/brightness"
POWER_RED="/sys/class/leds/bthomehubv5a:red:power/brightness"
if [ $INTERFACE == "wan" ]; then
case "$ACTION" in
ifup)
echo 0 > $BROADBAND_RED
;;
ifdown)
echo 255 > $BROADBAND_RED
;;
esac
fi

When DSL changes status it flashes accordingly.
Now instead it does nothing at all.

I would be nice, as I said, if you could put it back as it was. Or at least give an option to configure it as it was. Never mind the RGB LED. That is even better because I could mix the colours and have even more states reported as if it were more LEDs

thess:

@mathias - please comment so I can accept/deny re-open request.

TIA

mkresin:

Hey Ted,

my opinion on this hasn't changed and I don't see that any further discussion on this would lead to something useful (all caps demand). The change was intentionally and is not a bug.

The "workaround" provided by the reporter doesn't really work as outlined in my first response.

In the end it is a matter of personal preference and I provided everything that is required to restore the old LED behavior. The default for the whole Lantiq target is to indicate the ppp status in case only one LED for xdsl line state and internet connection exists.

ezplanet:

Mathias I would really appreciate if you could at least provide an option (either at compile time or runtime) to restore the LED functionality as it was with OpenWRT.
This at least would provide the end user a choice.

Thank you.

ezplanet:

I understand this isn't a bug. I requested to re-open this task as a feature.
We would like to have a choice a described above.

mkresin:

The feature you are requesting is already implemented. The way to go is to change /etc/config/system as outlined in my first reply. Not sure what else should be done.

It isn't something that is hardcoded. sbin/dsl_notify.sh checks for the existence of a led_dsl (in /etc/config/system). If the led exists, it will be used for line status indication.

It is just that LEDE has an initial default that you don't like. But the same applies to the default network config as well.

ezplanet:

There is actually a solution to this that allows using the only Broadband "b" LED of this router.

The current implementation of dsl_notify.sh assume that the target LED belongs to the ADSL status only. Instead we want the LED to be shared between the ADSL status and pppoa-wan status.

The way to implement it is to invert the ADSL LED status. So not connected becomes a solid colour, connected becomes OFF. Once connected, the ADSL LED is turned off and pppoa-wan status takes control and here we go. Having a solid or blinking (different colour) for pppoa-wan status implies also that ADSL is connected.

Assuming we assign RED to ADSL and BLUE to pppoa-wan we will have the following states:

ADSL disconnected: SOLID RED
ADSL handshake: RED slow blink
ADSL training: RED fast blink
ADSL UP: OFF
pppoa-wan disconnected: OFF
pppoa-wan connected SOLID BLUE
pppoa-wan tx/rx Blinking blue

The patch attached inverts the status of the ADSL LED on ADSL UP/DOWN thus allowing the following useful transitions:

RED (ADSL OFF) --> RED slow blink (HANDSHAKE) --> RED fast blink (TRAINING) --> OFF --> SOLID BLUE (pppoa-wan connected) and then Blinking blue if we tick the boxes for pppoa-wan tx rx status.

The ADSL is down, BLUE is OFF and RED comes ON, and we get a visual alert!

ezplanet:

Clearly the above patch is useful and applicable only to routers with one single LED available that we want to share for both ADSL and Internet connection status.

Routers with 2 separate LEDs will not need this patch.

Therefore we could provide a choice on weather to have dsl_notify.sh to invert the status of UP/DOWN within the configuration (firmware build configuration or alternatively software uci/luci configuration)

mkresin:

I'm getting really annoyed...

I guess it's time for you to accept what you are trying to do is just hackish to make something happen that doesn't really work.

I mean, having a single colour LED switched on in case something is not established is the worst user interaction I can think of. All devices I'm aware of are switching/having switched LEDs off if there is no link/sync/wireless/usb attached or whatever.

Additionally, the concept of triggers is that only one trigger controls the LED. The way you are doing it, the netdev trigger from /etc/config/system gets overwritten either by the none trigger (led_on()/led_off()) or by the timer trigger (led_timer()) while establishing the DSL connection. This clearly indicates that you never tested the patch with a device having only one LED for DSL/Internet (the Homehubs broadband LED is shown as three LEDs - one per colour - in the system).

Anyway, if you are still the opinion that your approach is worse the work, fix the outlined issues, follow the [[https://lede-project.org/docs/guide-developer/the-source-code#submitting_patches|submitting patches]] guide and provide something that covers all relevant files (dts, board.d etc.).

But please don't reopen this ticket again!

ezplanet:

I am using the patch I posted right now on BT Home Hub 5 Type A that has 3 colours defined for one LED. With the patch finally I can have the LED giving meaningful states for both ADSL and pppoa connections using two colours.

I am not impressed with the tone of your replies that discourages participation. You should accept that there are people with points of view and experience that differ from yours and you should make an effort to consider their input.

You are a "Project Manager" you should know better.

mkresin:

I am not impressed with the tone of your replies that discourages participation. You should accept that there are people with points of view and experience that differ from yours and you should make an effort to consider their input. You are a "Project Manager" you should know better.

Seriously? It's like talking to a wall... It's not that I don't consider other peoples input, it's just that I've explained you multiple times what is the issue and you still don't get it. Instead you propose broken and only single board tested solutions over and over again. What you are doing is wasting developer time, since I try to answer every request to encourage participation.

One last try. The HomeHub has "three" LEDs:

• bthomehubv5a:red:broadband
• bthomehubv5a:green:broadband
• bthomehubv5a:blue:broadband

Each of the LEDs can have a trigger. But the HH5a is special in that case. All other boards have a single LED:

• board:green:broadband

Which can only have one trigger. What you are proposing as solution would break exactly these boards, since the netdev trigger set during boot would be overwritten by the trigger set in dsl_notify.sh.

Yes, your hack might work for the HomeHub but it does break all other boards. Try to use only the bthomehubv5a:green:broadband for dsl line state and pppo* state to see what happens.

What you are really looking for is multicolour LED support in gpio-leds driver. Feel free to add multicolour led support to the gpio-leds driver and send your patch to the kernel people for inclusion.

But even that wouldn't work 100% perfect, since a disconnect of the pppo* connection would switch of the broadband LED albeit the underlying dsl line could be still in sync.

ezplanet:

ON THE BUG:

Further to my previous messages I discovered why your suggestion did not work.

When I create dsl_led via luci the following is inserted in /etc/config/system

config led
option default '0'
option name 'led_dsl'
option sysfs 'bthomehubv5a:red:broadband'
option trigger 'none'

/lib/functions.sh does not find dsl_led and thus the variable $led is not set in /sbin/dsl_notify.sh

It is only when I add manually the name to the led qualifier as follows:

config led 'led_dsl'

that finally $led is set correctly in /sbin/dsl_notify.sh

This looks to me as a bug.

ON THE LED FEATURE SPECIFIC TO BT HOME HUB 5:

I know that what I am suggesting applies to BT Home Hub 5 (and perhaps other boards) only. This is why I said in my previous post that this could be included as a "compile" or "configuration" only option.

"we could provide a choice on weather to have dsl_notify.sh to invert the status of UP/DOWN within the configuration (firmware build configuration or alternatively software uci/luci configuration)"

To support configurability I created a modified patch that re-uses dsl_led "default" parameter.
If the "default" is not set or set to '0' dsl_notify.sh behaves normally. If "default" is set to '1' it inverts the LED status on UP/DOWN.

I re-used the "default" parameter because it is already handled by luci and I did not want to venture into changing anything else. Using "default" set to '1' to invert the behaviour might be counter-intuitive, but it ensures that on a router with default values dsl_notify.sh behaves as per default.

A more elegant solution, if you prefer, would be to create a new "inverted" boolean parameter. But this would require changing also luci (I am not practical with that).

Please see below my system LED configuration, the new parametrized patch and a link to a video showing how the router behaves with the patch and configuration below applied.

config led
option default '0'
option name 'dmz'
option trigger 'netdev'
option dev 'eth1.2'
option mode 'tx rx'
option sysfs 'bthomehubv5a:green:wireless'


The video shows the LED behavior with the patch.
In the video, first you see adsl connected (BLUE "b"),
then I issue a /etc/init.d/dsl_control stop followed by a start
You see the LED going from BLUE to RED then blinking through ADSL states, the going OFF and turn BLUE when the pppoa is connected.

After that I issue a reboot to show the states during startup through to reconnection.

This satisfies my requirements because I have visual feedback covering all cases without having to log into the computer.

Please note that the patch I posted addresses only the BT Home Hub 5 inverted LED feature only. It does not fix the bug reported.

mkresin:

ON THE BUG:

Please do not call it a bug, if you are not doing what you were told to do. My example in the first post has "config led 'led_dsl'" from the beginning.

If LuCI adds an anonymous config instead of a named one, it might be a LuCI bug/limitation. It might be possible that dsl_notify.sh is using an outdated syntax. But since the named option is set by the ucidef_set_led_* base functions, I tend to point to a LuCI bug/limitation. You find the LuCI bugtracker here: https://github.com/openwrt/luci/issues. Nevertheless, create a new bugreport and do not report multiple issue in a single bugreport. This bugreport is about a complete different issue.

ON THE LED FEATURE SPECIFIC TO BT HOME HUB 5:

Please accept that we are not willing to add hacks and code/options - which need to be maintained by someone - just to workaround a limitation that needs to be fixed in the driver. I wrote already that adding support for multicolour LEDs to the gpio-leds driver is a "clean" solution. An alternate way could be to add support for multiple trigger + single LED.

It might be that you are fine with having such hacks. But please consider the amount of devices supported by LEDE. If only half of the devices would have such hacks, maintaining the code would be a nightmare. Some hacks should be kept in your private repository.

Furthermore, I wrote how to contribute by submit patches. Please follow the outlined way or accept that your patches are ignored.

If you want to discuss ideas, use the [[https://forum.lede-project.org|forum]] but do not abuse the bugtracker.