1380036 - Autophone - handle .profile and .bash_profile and fix adb issue in ap-* scripts

Status: RESOLVED FIXED

(Testing Graveyard :: Autophone, enhancement)

Description

[Bob Clary [:bc] (inactive)]

Ubuntu used .profile while Fedora uses .bash_profile. We need to update our ap-scripts to handle both.

Also, ever since we upgraded from adb 1.0.32, we've had problems with the ap-battery and ap-wifi commands not working properly. You can see the problem in the following output:

ap-getstate  | grep "=device" | sed -r "s|=device||" | while read n; do echo "[$n]"; serialno=$(ap-inventory --match=id=$n --output="%(serialno)s"); adb -s $serialno shell "dumpsys battery"; echo $?; done
[nexus-5-11]
nexus-5-4
nexus-5-7
pixel-11
pixel-12
Current Battery Service state:
  AC powered: false
  USB powered: true
  Wireless powered: false
  status: 5
  health: 2
  present: true
  level: 100
  scale: 100
  voltage: 4302
  current now: 71042
  temperature: 266
  technology: Li-ion
0

For some reason, invoking adb sequentially like this stops the loop and only invokes the command on one of the devices. I believe this is something to do with the way adb is implemented as a client/server. If we submit adb as a background job and wait for the child process, it will work however.

ap-getstate  | grep "=device" | sed -r "s|=device||" | while read n; do echo "[$n]"; serialno=$(ap-inventory --match=id=$n --output="%(serialno)s"); adb -s $serialno shell "dumpsys battery"& wait; done
[nexus-5-11]
Current Battery Service state:
  AC powered: false
  USB powered: true
  Wireless powered: false
  status: 5
  health: 2
  present: true
  level: 100
  scale: 100
  voltage: 4302
  current now: 71042
  temperature: 266
  technology: Li-ion
[nexus-5-4]
Current Battery Service state:
  AC powered: false
  USB powered: true
  Wireless powered: false
  status: 5
  health: 2
  present: true
  level: 100
  scale: 100
  voltage: 4353
  current now: 2127
  temperature: 231
  technology: Li-ion
[nexus-5-7]
Current Battery Service state:
  AC powered: false
  USB powered: true
  Wireless powered: false
  status: 5
  health: 2
  present: true
  level: 100
  scale: 100
  voltage: 4312
  current now: 1187
  temperature: 245
  technology: Li-ion
[pixel-11]
Current Battery Service state:
  AC powered: false
  USB powered: true
  Wireless powered: false
  Max charging current: 900000
  Max charging voltage: 5000000
  Charge counter: 5110182
  status: 5
  health: 2
  present: true
  level: 100
  scale: 100
  voltage: 4359
  temperature: 267
  technology: Li-ion
[pixel-12]
Current Battery Service state:
  AC powered: false
  USB powered: true
  Wireless powered: false
  Max charging current: 900000
  Max charging voltage: 5000000
  Charge counter: 3326777
  status: 5
  health: 2
  present: true
  level: 100
  scale: 100
  voltage: 4357
  temperature: 267
  technology: Li-ion

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: bug-1380036-v1.patch

Comment 2

[Geoff Brown [:gbrown]]

Comment on attachment 8885342 [details] [diff] [review]
bug-1380036-v1.patch

Review of attachment 8885342 [details] [diff] [review]:
-----------------------------------------------------------------

::: ap-battery
@@ +5,5 @@
>          echo ============== $n ===========
> +        # Work around failures sequentially invoking adb by placing
> +        # job in background and waiting for the child process.
> +        adb -s $(ap-inventory --match=id=$n --output="%(serialno)s") shell "dumpsys battery" &
> +        wait

This feels wrong...but if the "right" way doesn't work...

::: ap-wifi
@@ +9,5 @@
>              interface=wlan0
>          fi
> +        # Work around failures sequentially invoking adb by placing
> +        # job in background and waiting for the child process.
> +        adb -s $(ap-inventory --match=id=$n --output="%(serialno)s") shell "uptime; ifconfig $interface" &

Do you recall why wait-for-device was here?

Comment 3

[Bob Clary [:bc] (inactive)]

(In reply to Geoff Brown [:gbrown] from comment #2)
> Comment on attachment 8885342 [details] [diff] [review]
> bug-1380036-v1.patch
> 
> Review of attachment 8885342 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: ap-battery
> @@ +5,5 @@
> >          echo ============== $n ===========
> > +        # Work around failures sequentially invoking adb by placing
> > +        # job in background and waiting for the child process.
> > +        adb -s $(ap-inventory --match=id=$n --output="%(serialno)s") shell "dumpsys battery" &
> > +        wait
> 
> This feels wrong...but if the "right" way doesn't work...
> 

Agreed. I've been living with this for a while and finally got tired enough to do something about it. It does work though but that is all I can say for it.

> ::: ap-wifi
> @@ +9,5 @@
> >              interface=wlan0
> >          fi
> > +        # Work around failures sequentially invoking adb by placing
> > +        # job in background and waiting for the child process.
> > +        adb -s $(ap-inventory --match=id=$n --output="%(serialno)s") shell "uptime; ifconfig $interface" &
> 
> Do you recall why wait-for-device was here?

No. I removed it for consistency sake since I didn't have it elsewhere. The worst case it it would fail to handle the device if it wasn't available.

Thanks!

Comment 4

[Bob Clary [:bc] (inactive)]

https://github.com/mozilla/autophone/commit/1fafb6eb6262a0abd9b2381ab7d9c512743c8653