This repository has been archived by the owner. It is now read-only.

Add a maximum-attention hook to help with debugging boot issues #1238

Merged
merged 1 commit into from Feb 22, 2018

Conversation

Projects
None yet
4 participants
@MayeulC
Contributor

MayeulC commented Feb 19, 2018

The hook flashes every LED it can find, as well as the vibration motor.
More paths are probably necessary to support more devices and kernels.

This is intended as a first approach to #1217
Also somewhat related to #88

The name comes from ollieparanoid on Matrix, IIRC.

state=false # false = off, true=on
while :
do

This comment has been minimized.

@MayeulC

MayeulC Feb 19, 2018

Contributor

OK, there might be a stray whiteline here

@MartijnBraam

It works correctly on my hammerhead, it flashes the camera led and the vibration motor works

@MartijnBraam

This comment has been minimized.

Member

MartijnBraam commented Feb 20, 2018

Hmm it doesn't seem to work with the notification led on my hammerhead, but that led has a slightly more complicated sysfs interface. To turn that one do:

cat max_brightness > brightness
echo 100 0 > on_off_ms
# You have to write a different value to rgb_start for every change
echo 1 > rgb_start

You can also make the kernel blink the led instead of the script by setting on_off_ms to the blinking times

echo 2000 2000 > on_off_ms

I guess you can just check if the current led has a rgb_start file to do this procedure before entering the blinking loop.

For reference my ssh session:

localhost:/sys/devices/leds-qpnp-23/leds/red# cat max_brightness > brightness^C
localhost:/sys/devices/leds-qpnp-23/leds/red# cat on_off_ms 
50 50
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 50 50 > on_off_ms 
localhost:/sys/devices/leds-qpnp-23/leds/red# cat rgb_start 
0
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 1 > rgb_start 
localhost:/sys/devices/leds-qpnp-23/leds/red# cat rgb_start ^C
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 100 0 > on_off_ms 
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 1 > rgb_start 
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 2 > rgb_start 
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 2000 2000 > on_off_ms 
localhost:/sys/devices/leds-qpnp-23/leds/red# echo 3 > rgb_start 
localhost:/sys/devices/leds-qpnp-23/leds/red# cd ..
localhost:/sys/devices/leds-qpnp-23/leds# cd blue/
localhost:/sys/devices/leds-qpnp-23/leds/blue# ls
brightness      duty_pcts       max_brightness  pause_hi        power           rgb_start       subsystem       uevent
device          lut_flags       on_off_ms       pause_lo        ramp_step_ms    start_idx       trigger
localhost:/sys/devices/leds-qpnp-23/leds/blue# cat max_brightness > brightness 
localhost:/sys/devices/leds-qpnp-23/leds/blue# echo 1000 1000 > on_off_ms 
localhost:/sys/devices/leds-qpnp-23/leds/blue# echo 343 > rgb_start 
localhost:/sys/devices/leds-qpnp-23/leds/blue# cd ..
localhost:/sys/devices/leds-qpnp-23/leds# cd green/
localhost:/sys/devices/leds-qpnp-23/leds/green# cat max_brightness > brightness 
localhost:/sys/devices/leds-qpnp-23/leds/green# echo 1500 1500 > on_off_ms 
localhost:/sys/devices/leds-qpnp-23/leds/green# echo 3232 > rgb_start 
localhost:/sys/devices/leds-qpnp-23/leds/green# 
@MayeulC

This comment has been minimized.

Contributor

MayeulC commented Feb 20, 2018

Thanks a lot for your review, @MartijnBraam. THere are a few things there (hence why I called it a first approach):

  • The LCD backlight should flash as well (if applicable), but I am not sure whether I should add a new splash to make it more visible
  • currently, the hook loops forever. I am not sure if it should continue the boot process or not, I think it is better this way, as it is just intended as a one-off to diagnose boot issues
  • There seem to be a lot of different vendor interfaces out there. I didn't really want to support non-standard ones, but I guess it would make the porting process easier.

I didn't really understand the purpose of rgb_start here, though.

@drebrez

This comment has been minimized.

Member

drebrez commented Feb 20, 2018

@MayeulC when I get home I will try on the various devices I have and let you know the results

@MartijnBraam

This comment has been minimized.

Member

MartijnBraam commented Feb 20, 2018

The backlight goes to full brightness and then turns of, that is probably a sideeffect of some fbdev code in the kernel but the led does get controlled once.

Also the rgb led is slightly weird but the rgb_start file is to make the new settings apply all at once since you have to change 2 (or more) settings and accros 3 led devices usually if you wan't to flash a non-primary color.

I think this hook should support as many leds as possible since it is for debugging booting issues.

@drebrez

This comment has been minimized.

Member

drebrez commented Feb 20, 2018

Tested and here are the results:

device display backlight button backlight front led camera led vibrator
samsung-i9070 NO YES - NO YES
samsung-maguro YES NO NO NO YES
htc-bravo YES YES YES YES YES
samsung-klte YES YES YES YES YES
huawei-y530 NO YES YES NO YES
lg-dory YES - - - YES

So for me it's a good feedback 👍

@ollieparanoid

Code looks good to me! I think it's good that you loop forever when the hook is enabled.

@MayeulC: should we merge this as initial version, or would you like to add support for more exotic devices first? In my opinion it makes sense to support as many devices as possible, but in the current version it is useful already (chances are high that either the LEDs will blink or the vibrator will work).

Thanks for making this, and thanks for the extensive testing everyone! \o/

}
find_vibrator()
{

This comment has been minimized.

@ollieparanoid

ollieparanoid Feb 21, 2018

Member

How about find_vibrator() { for consistency?

This comment has been minimized.

@MayeulC

MayeulC Feb 21, 2018

Contributor

Yes, you are right, I am just soo used to my usual C syntax that I tend to mix things up. Thanks for noticing!

# vibrate_loop vibrates each VIBRATION_INTERVAL for VIBRATION_DURATION
# it takes a timed_device path to the vibrator as $1
vibrate_loop()

This comment has been minimized.

@ollieparanoid

ollieparanoid Feb 21, 2018

Member

same here

Add a maximum-attention hook to help with debugging boot issues
The hook flashes every LED it can find, as well as the vibration motor.
More paths are probably necessary to support more devices and kernels.
@MayeulC

This comment has been minimized.

Contributor

MayeulC commented Feb 21, 2018

@ollieparanoid, I fixed the formatting issue (thanks for noticing! I have C-style formatting hardcoded in my fingers).

I think this should get merged, I will (maybe with some others) think about a way to add more device in a way that's as generic as possible, but will probably be busy for a few days.

@ollieparanoid ollieparanoid merged commit a261a1c into postmarketOS:master Feb 22, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 75.248%
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.