[Merge] lp:~mandel/ubuntu-location-provider-here/work-with-locked-sims into lp:ubuntu-location-provider-here

Thomas Voß thomas.voss at canonical.com
Mon Sep 15 10:10:22 UTC 2014


Review: Needs Fixing



Diff comments:

> === modified file 'debian/changelog'
> --- debian/changelog	2014-09-01 20:36:25 +0000
> +++ debian/changelog	2014-09-05 12:58:06 +0000
> @@ -1,3 +1,10 @@
> +ubuntu-location-provider-here (0.1+14.10.20140829-0ubuntu2) UNRELEASED; urgency=medium
> +
> +  * Improve the upstart jobs so that no loops are needed and whoopsie is not
> +    required.
> +
> + -- Manuel de la Pena <manuel.delapena at canonical.com>  Fri, 05 Sep 2014 13:18:00 +0200
> +
>  ubuntu-location-provider-here (0.1+14.10.20140829-0ubuntu1) utopic; urgency=medium
>  
>    * Initial release.
> 
> === added directory 'etc/dbus-1'
> === removed directory 'etc/dbus-1'
> === added directory 'etc/dbus-1/system.d'
> === removed directory 'etc/dbus-1/system.d'
> === added file 'etc/dbus-1/system.d/com.here.posclientd.conf'
> --- etc/dbus-1/system.d/com.here.posclientd.conf	1970-01-01 00:00:00 +0000
> +++ etc/dbus-1/system.d/com.here.posclientd.conf	2014-09-05 12:58:06 +0000
> @@ -0,0 +1,32 @@
> +<?xml version="1.0"?>
> +<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
> + "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
> +
> +<busconfig>
> +        <policy user="phablet">
> +                <deny own="com.here.posclientd"/>
> +
> +                <allow send_destination="com.here.posclientd"/>
> +                <allow send_interface="com.here.posclientd"/>
> +
> +                <allow receive_sender="com.here.posclientd" receive_type="signal"/>
> +        </policy>
> +
> +	<policy user="root">
> +		<allow own="com.here.posclientd"/>
> +
> +		<allow send_destination="com.here.posclientd"/>
> +		<allow send_interface="com.here.posclientd"/>
> +
> +		<allow receive_sender="com.here.posclientd" receive_type="signal"/>
> +	</policy>	
> +
> +	<policy context="default">
> +		<deny own="com.here.posclientd"/>
> +
> +		<deny send_destination="com.here.posclientd"/>
> +		<deny send_interface="com.here.posclientd"/>
> +
> +		<deny receive_sender="com.here.posclientd" receive_type="signal"/>
> +	</policy>
> +</busconfig>
> 
> === removed file 'etc/dbus-1/system.d/com.here.posclientd.conf'
> --- etc/dbus-1/system.d/com.here.posclientd.conf	2014-08-29 19:45:58 +0000
> +++ etc/dbus-1/system.d/com.here.posclientd.conf	1970-01-01 00:00:00 +0000
> @@ -1,32 +0,0 @@
> -<?xml version="1.0"?>
> -<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
> - "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
> -
> -<busconfig>
> -        <policy user="phablet">
> -                <deny own="com.here.posclientd"/>
> -
> -                <allow send_destination="com.here.posclientd"/>
> -                <allow send_interface="com.here.posclientd"/>
> -
> -                <allow receive_sender="com.here.posclientd" receive_type="signal"/>
> -        </policy>
> -
> -	<policy user="root">
> -		<allow own="com.here.posclientd"/>
> -
> -		<allow send_destination="com.here.posclientd"/>
> -		<allow send_interface="com.here.posclientd"/>
> -
> -		<allow receive_sender="com.here.posclientd" receive_type="signal"/>
> -	</policy>	
> -
> -	<policy context="default">
> -		<deny own="com.here.posclientd"/>
> -
> -		<deny send_destination="com.here.posclientd"/>
> -		<deny send_interface="com.here.posclientd"/>
> -
> -		<deny receive_sender="com.here.posclientd" receive_type="signal"/>
> -	</policy>
> -</busconfig>
> 
> === added file 'etc/dbus-1/system.d/com.here.slpgwd.conf'
> --- etc/dbus-1/system.d/com.here.slpgwd.conf	1970-01-01 00:00:00 +0000
> +++ etc/dbus-1/system.d/com.here.slpgwd.conf	2014-09-05 12:58:06 +0000
> @@ -0,0 +1,23 @@
> +<?xml version="1.0"?>
> +<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
> + "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
> +
> +<busconfig>
> +	<policy user="root">
> +		<allow own="com.here.slpgwd"/>
> +
> +		<allow send_destination="com.here.slpgwd"/>
> +		<allow send_interface="com.here.slpgwd"/>
> +
> +		<allow receive_sender="com.here.slpgwd" receive_type="signal"/>
> +	</policy>	
> +
> +	<policy context="default">
> +		<deny own="com.here.slpgwd"/>
> +
> +		<deny send_destination="com.here.slpgwd"/>
> +		<deny send_interface="com.here.slpgwd"/>
> +
> +		<deny receive_sender="com.here.slpgwd" receive_type="signal"/>
> +	</policy>
> +</busconfig>
> 
> === removed file 'etc/dbus-1/system.d/com.here.slpgwd.conf'
> --- etc/dbus-1/system.d/com.here.slpgwd.conf	2014-08-29 19:45:58 +0000
> +++ etc/dbus-1/system.d/com.here.slpgwd.conf	1970-01-01 00:00:00 +0000
> @@ -1,23 +0,0 @@
> -<?xml version="1.0"?>
> -<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
> - "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
> -
> -<busconfig>
> -	<policy user="root">
> -		<allow own="com.here.slpgwd"/>
> -
> -		<allow send_destination="com.here.slpgwd"/>
> -		<allow send_interface="com.here.slpgwd"/>
> -
> -		<allow receive_sender="com.here.slpgwd" receive_type="signal"/>
> -	</policy>	
> -
> -	<policy context="default">
> -		<deny own="com.here.slpgwd"/>
> -
> -		<deny send_destination="com.here.slpgwd"/>
> -		<deny send_interface="com.here.slpgwd"/>
> -
> -		<deny receive_sender="com.here.slpgwd" receive_type="signal"/>
> -	</policy>
> -</busconfig>
> 
> === added file 'etc/dbus-1/system.d/com.ubuntu.espoo.Service.conf'
> --- etc/dbus-1/system.d/com.ubuntu.espoo.Service.conf	1970-01-01 00:00:00 +0000
> +++ etc/dbus-1/system.d/com.ubuntu.espoo.Service.conf	2014-09-05 12:58:06 +0000
> @@ -0,0 +1,17 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +
> +<!DOCTYPE busconfig PUBLIC
> + "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
> + "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
> +<busconfig>
> +    <policy user="root">
> +       <allow own="com.ubuntu.espoo.Service.Provider"/>
> +       <allow send_destination="com.ubuntu.espoo.Service.Provider"/>
> +       <allow send_destination="com.ubuntu.espoo.Service.Provider"
> +              send_interface="org.freedesktop.DBus.Introspectable"/>
> +    </policy>
> +
> +    <policy context="default">
> +    </policy>
> +
> +</busconfig>
> 
> === removed file 'etc/dbus-1/system.d/com.ubuntu.espoo.Service.conf'
> --- etc/dbus-1/system.d/com.ubuntu.espoo.Service.conf	2014-08-29 19:45:58 +0000
> +++ etc/dbus-1/system.d/com.ubuntu.espoo.Service.conf	1970-01-01 00:00:00 +0000
> @@ -1,17 +0,0 @@
> -<?xml version="1.0" encoding="UTF-8"?>
> -
> -<!DOCTYPE busconfig PUBLIC
> - "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
> - "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
> -<busconfig>
> -    <policy user="root">
> -       <allow own="com.ubuntu.espoo.Service.Provider"/>
> -       <allow send_destination="com.ubuntu.espoo.Service.Provider"/>
> -       <allow send_destination="com.ubuntu.espoo.Service.Provider"
> -              send_interface="org.freedesktop.DBus.Introspectable"/>
> -    </policy>
> -
> -    <policy context="default">
> -    </policy>
> -
> -</busconfig>
> 
> === modified file 'etc/init/ubuntu-espoo-service.conf'
> --- etc/init/ubuntu-espoo-service.conf	2014-09-01 20:24:15 +0000
> +++ etc/init/ubuntu-espoo-service.conf	2014-09-05 12:58:06 +0000
> @@ -1,12 +1,12 @@
>  description "Ubuntu Espoo Position Provider"
>  
> -start on started dbus and started ubuntu-location-service and started whoopsie
> +start on started dbus and started ubuntu-location-service
>  
>  respawn
>  
>  env BASE="/custom/vendor/here/location-provider"
>  
> -pre-start script
> +script
>      if ! [ -x "$BASE/bin/@DEB_HOST_MULTIARCH@/ubuntu-espoo-service" ]; then
>          stop
>      fi
> @@ -15,10 +15,7 @@
>      if ! herepositioning-license-accepted; then
>          stop
>      fi
> -end script
>  
> -script
> -    LD_LIBRARY_PATH="$BASE/lib/@DEB_HOST_MULTIARCH@"
> -    export LD_LIBRARY_PATH
> +    export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$BASE/lib/@DEB_HOST_MULTIARCH@"
>      exec "$BASE/bin/@DEB_HOST_MULTIARCH@/ubuntu-espoo-service"
>  end script
> 
> === modified file 'etc/init/ubuntu-location-provider-here-posclientd.conf'
> --- etc/init/ubuntu-location-provider-here-posclientd.conf	2014-09-01 20:24:15 +0000
> +++ etc/init/ubuntu-location-provider-here-posclientd.conf	2014-09-05 12:58:06 +0000
> @@ -1,4 +1,4 @@
> -start on started dbus and started ofono and started whoopsie
> +start on started dbus and started ofono
>  
>  respawn
>  
> @@ -7,31 +7,7 @@
>  env BASE="/custom/vendor/here/location-provider"
>  env STORAGE="/userdata/system-data/var/lib/ubuntu-location-provider-here"
>  
> -# set the env vars to be empty so that they are always present and then provide
> -# the required data
> -
> -env HERE_SIM_MNC=""
> -export HERE_SIM_MNC
> -
> -env HERE_SIM_MCC=""
> -export HERE_SIM_MCC
> -
> -env HERE_UNIQUE_DEVICE_ID=""
> -export HERE_UNIQUE_DEVICE_ID
> -
> -env HERE_IMEI_FRAGMENT=""
> -export HERE_IMEI_FRAGMENT
> -
> -env HERE_DEVICE_MANUFACTURER=""
> -export HERE_DEVICE_MANUFACTURER
> -
> -env HERE_DEVICE_FIRMWARE_VERSION=""
> -export HERE_DEVICE_FIRMWARE_VERSION
> -
> -env HERE_DEVICE_TERMINAL_MODEL=""
> -export HERE_DEVICE_TERMINAL_MODEL
> -
> -pre-start script
> +script
>      if ! [ -x "$BASE/bin/@DEB_HOST_MULTIARCH@/posclientd" ]; then
>          stop
>      fi
> @@ -40,48 +16,12 @@
>      if ! herepositioning-license-accepted; then
>          stop
>      fi
> -end script
> -
> -script
> -    # try to set the values needed by each of the env variables before we start
> -    # the here processes
> -
> -    DBUS_METHOD="org.ofono.NetworkRegistration.GetProperties"
> -    DBUS_CMD="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono /ril_0 $DBUS_METHOD || true)"
> -    CODES="MobileCountryCode MobileNetworkCode"
> -
> -    for INFO in $CODES; do
> -        VAL=$(echo "$DBUS_CMD"| grep $INFO| sed -e 's/\(^[\.0-9]*\)[^.0-9]*/\1/g' -e 's/ .*$//')
> -        eval $INFO=$VAL
> -    done
> -
> -    # set the ofono env vars
> -    export HERE_SIM_MCC=$MobileCountryCode
> -    export HERE_SIM_MNC=$MobileNetworkCode
> -
> -    DBUS_METHOD="org.ofono.Modem.GetProperties"
> -    DBUS_CMD="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono /ril_0 $DBUS_METHOD)"
> -    CODES="Serial"
> -
> -    for INFO in $CODES; do
> -        VAL=$(echo "$DBUS_CMD"| grep $INFO| sed -e 's/\(^[\.0-9]*\)[^.0-9]*/\1/g' -e 's/ .*$//' | awk '{print substr($0,1,8)}')
> -        eval $INFO=$VAL
> -    done
> -
> -    export HERE_IMEI_FRAGMENT=$Serial
> -
> -    # set the unique identifier
> -    UNIQUE_ID=$(sudo  gdbus call -y -d com.ubuntu.WhoopsiePreferences -o /com/ubuntu/WhoopsiePreferences -m com.ubuntu.WhoopsiePreferences.GetIdentifier | sed "s/['(),]//g")
> -    export HERE_UNIQUE_DEVICE_ID=$UNIQUE_ID
> -
> -    export HERE_DEVICE_TERMINAL_MODEL="$(getprop ro.product.model)"
> -    export HERE_DEVICE_MANUFACTURER="$(getprop ro.product.manufacturer)"
> -    export HERE_DEVICE_FIRMWARE_VERSION="$(getprop ro.build.version.incremental)"
> +
> +    # set the env variables
> +    exec "/usr/lib/@DEB_HOST_MULTIARCH@/ubuntu-location-provider-here/set-enviroment.sh"
>  
>      mkdir -p "$STORAGE"
>  
> -    LD_LIBRARY_PATH="$BASE/lib/@DEB_HOST_MULTIARCH@"
> -    export LD_LIBRARY_PATH
> +    export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$BASE/lib/@DEB_HOST_MULTIARCH@"
>      exec "$BASE/bin/@DEB_HOST_MULTIARCH@/posclientd" --preinst-dir "$BASE/share" --storage-dir "$STORAGE"
> -
>  end script
> 
> === added file 'etc/init/ubuntu-location-provider-here-sim-watcher.conf'
> === modified file 'etc/init/ubuntu-location-provider-here-slpgwd.conf'
> --- etc/init/ubuntu-location-provider-here-slpgwd.conf	2014-09-01 20:24:15 +0000
> +++ etc/init/ubuntu-location-provider-here-slpgwd.conf	2014-09-05 12:58:06 +0000
> @@ -1,4 +1,4 @@
> -start on started dbus and started ofono and started whoopsie
> +start on started dbus and started ofono
>  
>  respawn
>  
> @@ -7,31 +7,7 @@
>  env BASE="/custom/vendor/here/location-provider"
>  env STORAGE=/userdata/system-data/var/lib/ubuntu-location-provider-here
>  
> -# set the env vars to be empty so that they are always present and then provide
> -# the required data
> -
> -env HERE_SIM_MNC=""
> -export HERE_SIM_MNC
> -
> -env HERE_SIM_MCC=""
> -export HERE_SIM_MCC
> -
> -env HERE_UNIQUE_DEVICE_ID=""
> -export HERE_UNIQUE_DEVICE_ID
> -
> -env HERE_IMEI_FRAGMENT=""
> -export HERE_IMEI_FRAGMENT
> -
> -env HERE_DEVICE_MANUFACTURER=""
> -export HERE_DEVICE_MANUFACTURER
> -
> -env HERE_DEVICE_FIRMWARE_VERSION=""
> -export HERE_DEVICE_FIRMWARE_VERSION
> -
> -env HERE_DEVICE_TERMINAL_MODEL=""
> -export HERE_DEVICE_TERMINAL_MODEL
> -
> -pre-start script
> +script
>      if ! [ -x "$BASE/bin/@DEB_HOST_MULTIARCH@/slpgwd" ]; then
>          stop
>      fi
> @@ -40,48 +16,9 @@
>      if ! herepositioning-license-accepted; then
>          stop
>      fi
> -end script
> -
> -script
> -    # try to set the values needed by each of the env variables before we start
> -    # the here processes
> -
> -    DBUS_METHOD="org.ofono.NetworkRegistration.GetProperties"
> -    DBUS_CMD="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono /ril_0 $DBUS_METHOD || true)"
> -    CODES="MobileCountryCode MobileNetworkCode"
> -
> -    for INFO in $CODES; do
> -        VAL=$(echo "$DBUS_CMD"| grep $INFO| sed -e 's/\(^[\.0-9]*\)[^.0-9]*/\1/g' -e 's/ .*$//')
> -        eval $INFO=$VAL
> -    done
> -
> -    # set the ofono env vars
> -    export HERE_SIM_MCC=$MobileCountryCode
> -    export HERE_SIM_MNC=$MobileNetworkCode
> -
> -    DBUS_METHOD="org.ofono.Modem.GetProperties"
> -    DBUS_CMD="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono /ril_0 $DBUS_METHOD)"
> -    CODES="Serial"
> -
> -    for INFO in $CODES; do
> -        VAL=$(echo "$DBUS_CMD"| grep $INFO| sed -e 's/\(^[\.0-9]*\)[^.0-9]*/\1/g' -e 's/ .*$//' | awk '{print substr($0,1,8)}')
> -        eval $INFO=$VAL
> -    done
> -
> -    export HERE_IMEI_FRAGMENT=$Serial
> -
> -    # set the unique identifier
> -    UNIQUE_ID=$(sudo  gdbus call -y -d com.ubuntu.WhoopsiePreferences -o /com/ubuntu/WhoopsiePreferences -m com.ubuntu.WhoopsiePreferences.GetIdentifier | sed "s/['(),]//g")
> -    export HERE_UNIQUE_DEVICE_ID=$UNIQUE_ID
> -
> -    export HERE_DEVICE_TERMINAL_MODEL="$(getprop ro.product.model)"
> -    export HERE_DEVICE_MANUFACTURER="$(getprop ro.product.manufacturer)"
> -    export HERE_DEVICE_FIRMWARE_VERSION="$(getprop ro.build.version.incremental)"
>  
>      mkdir -p "$STORAGE"
>  
> -    LD_LIBRARY_PATH="$BASE/lib/@DEB_HOST_MULTIARCH@"
> -    export LD_LIBRARY_PATH
> +    export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$BASE/lib/@DEB_HOST_MULTIARCH@"
>      exec "$BASE/bin/@DEB_HOST_MULTIARCH@/slpgwd" --preinst-dir "$BASE/share" --storage-dir "$STORAGE"
> -
>  end script
> 
> === added directory 'usr/lib'
> === added directory 'usr/lib/ubuntu-location-provider-here'
> === added file 'usr/lib/ubuntu-location-provider-here/set-enviroment.sh'
> --- usr/lib/ubuntu-location-provider-here/set-enviroment.sh	1970-01-01 00:00:00 +0000
> +++ usr/lib/ubuntu-location-provider-here/set-enviroment.sh	2014-09-05 12:58:06 +0000
> @@ -0,0 +1,35 @@
> +#!/bin/sh
> +
> +# We support phones with more than one modem and therefore we need to know how many
> +# of them we have to check if we are online in each of them
> +DBUS_SEND="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono / org.ofono.Manager.GetModems || true)"
> +modem=$(echo "$DBUS_SEND" | grep -o /ril_[0-9]* | sort -V | tr "\n" " " | cut -d " " -f1)  # just grab the first modem
> +
> +if [ "$modem" = "" ]; then
> +    echo "ERROR: No modem has been found!";
> +    exit 1;
> +fi
> +                 
> + # we do not need to get online to get the following information
> +DBUS_SEND="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono $modem org.ofono.Modem.GetProperties || true)"
> + 
> +export HERE_IMEI_FRAGMENT=$(echo "$DBUS_SEND" | grep Serial | grep -o '[0-9]*' | cut -c 1-8)
> +export HERE_UNIQUE_DEVICE_ID="$(dbus-send --system --print-reply=literal --type=method_call --dest=com.ubuntu.WhoopsiePreferences /com/ubuntu/WhoopsiePreferences com.ubuntu.WhoopsiePreferences.GetIdentifier | sed -e 's/^[ \t]*//' || true)"

What happens if whoopsie is not up at this point?

> +     
> +# TODO: ofono should be used for this but atm there is a bug and it does not export the right values

Could you please provide a link to the respective bug report here?

> +export HERE_DEVICE_TERMINAL_MODEL="$(getprop ro.product.model)"
> +export HERE_DEVICE_MANUFACTURER="$(getprop ro.product.manufacturer)"
> +export HERE_DEVICE_FIRMWARE_VERSION="$(getprop ro.build.version.incremental)"
> +     
> +# the SimManager can fail in some devices because it does not appear if there is no sim present. In that case we export the env vars empty
> +DBUS_SEND="$(dbus-send --system --print-reply=literal --type=method_call --dest=org.ofono $modem org.ofono.SimManager.GetProperties || true)"
> +  
> +if [ ! "$DBUS_SEND" = "" ]; then
> +    export HERE_SIM_MCC=$(echo "$DBUS_SEND" | grep MobileCountryCode | grep -o '[0-9]\{3\}')
> +    export HERE_SIM_MNC=$(echo "$DBUS_SEND" | grep MobileNetworkCode | grep -o '[0-9]\{2\}')
> +else
> +    export HERE_SIM_MCC=""
> +    export HERE_SIM_MNC=""
> +fi
> +
> +exit 0
> 


-- 
https://code.launchpad.net/~mandel/ubuntu-location-provider-here/work-with-locked-sims/+merge/233204
Your team Ubuntu Phablet Team is subscribed to branch lp:ubuntu-location-provider-here.



More information about the Ubuntu-reviews mailing list