Merge lp:~jonas-drange/ubuntu-push/control-poller into lp:ubuntu-push/automatic

[Jonas G. Drange]

[ubuntu-push-client/poller] assert whether or not there's a connection using the same method as local connectivity package

[ubuntu-push-client/poller] assert whether or not there's a connection using the same method as local connectivity package [1].

[1] http://bazaar.launchpad.net/~ubuntu-push-hackers/ubuntu-push/trunk/view/150/bus/connectivity/connectivity.go#L197

[Bill Filler (bfiller)]

testing branch on arale, seems great. I've tried different combinations of wifi and cellular data on/off and get the expected results with polling not being attempted if no connection.

I am seeing a problem on my krillin with the following setup:
wifi off, cellular data on, I have a connection (Edge), but no data left on plan. It polls for 1 minute in this case and repeats every 30seconds (I changed poll_interval to 30s)

wonder if there is a way to detect if we can actually send recv/data, rather than detecting a connection only

[Samuele Pedroni (pedronis)]

bfiller:
> wonder if there is a way to detect if we can actually send recv/data, rather than detecting a
> connection only

we have code a bit like that in

bus/connectivity

it plays a role indirectly here not in control() but step(), IsConnected() invoked there ultimately depends on the state found out through connecivity, the issue there that you need to be awake and have a bit of time to check recv/data... unless there some low level way that doesn't involve actually hitting some remote service,

as I mentioning ideally there would be one service in the system that decided "really connected/not connected", and push client would just trust it (then we could simplify code here and code in/depending on bus/connectivity)

[Samuele Pedroni (pedronis)]

looks good code wise, don't know/cannot judge if this bit of NM works correctly across all devices (given my past troubles with fligh mode and it for example :) ), I let that be tested/opined on by other teams

review: Approve

[Jonas G. Drange (jonas-drange)]

Test plan (edited) completed, all pass.

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~jonas-drange/ubuntu-push/control-poller into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
scripts/deps.sh ubuntu-push-client.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push launchpad.net/ubuntu-push/accounts launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/accounts launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/emblemcounter launchpad.net/ubuntu-push/bus/haptic launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/polld launchpad.net/ubuntu-push/bus/powerd launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/unitygreeter launchpad.net/ubuntu-push/bus/urfkill launchpad.net/ubuntu-push/bus/windowstack launchpad.net/ubuntu-push/click launchpad.net/ubuntu-push/click/cappinfo launchpad.net/ubuntu-push/click/cblacklist launchpad.net/ubuntu-push/click/cclick launchpad.net/ubuntu-push/click/testing launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/identifier launchpad.net/ubuntu-push/identifier/testing launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/launch_helper/cual launchpad.net/ubuntu-push/launch_helper/helper_finder launchpad.net/ubuntu-push/launch_helper/legacy launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/poller launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/sounds launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/urldispatcher launchpad.net/ubuntu-push/urldispatcher/curldispatcher launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/ launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-...

[lp:~jonas-drange/ubuntu-push/control-poller]

413. By Jonas G. Drange on 2015-09-28

make format

=== modified file 'poller/poller.go'
--- poller/poller.go	2015-04-29 15:22:03 +0000
+++ poller/poller.go	2015-09-28 08:25:01 +0000
@@ -25,9 +25,9 @@
 	"time"
 
 	"launchpad.net/ubuntu-push/bus"
+	"launchpad.net/ubuntu-push/bus/networkmanager"
 	"launchpad.net/ubuntu-push/bus/polld"
 	"launchpad.net/ubuntu-push/bus/powerd"
-	"launchpad.net/ubuntu-push/bus/urfkill"
 	"launchpad.net/ubuntu-push/client/session"
 	"launchpad.net/ubuntu-push/logger"
 	"launchpad.net/ubuntu-push/util"
@@ -69,7 +69,7 @@
 	log                  logger.Logger
 	powerd               powerd.Powerd
 	polld                polld.Polld
-	urfkill              urfkill.URfkill
+	nm                   networkmanager.NetworkManager
 	cookie               string
 	sessionState         stater
 	requestWakeupCh      chan struct{}
@@ -101,12 +101,13 @@
 	if p.powerd != nil || p.polld != nil {
 		return ErrAlreadyStarted
 	}
+
 	powerdEndp := bus.SystemBus.Endpoint(powerd.BusAddress, p.log)
 	polldEndp := bus.SessionBus.Endpoint(polld.BusAddress, p.log)
-	urEndp := bus.SystemBus.Endpoint(urfkill.BusAddress, p.log)
-	urWLANKillswitchEndp := bus.SystemBus.Endpoint(urfkill.WLANKillswitchBusAddress, p.log)
+	nmEndp := bus.SystemBus.Endpoint(networkmanager.BusAddress, p.log)
+
 	var wg sync.WaitGroup
-	wg.Add(4)
+	wg.Add(3)
 	go func() {
 		n := util.NewAutoRedialer(powerdEndp).Redial()
 		p.log.Debugf("powerd dialed on try %d", n)
@@ -118,20 +119,15 @@
 		wg.Done()
 	}()
 	go func() {
-		n := util.NewAutoRedialer(urEndp).Redial()
-		p.log.Debugf("URfkill dialed on try %d", n)
-		wg.Done()
-	}()
-	go func() {
-		n := util.NewAutoRedialer(urWLANKillswitchEndp).Redial()
-		p.log.Debugf("URfkill (WLAN killswitch) dialed on try %d", n)
+		n := util.NewAutoRedialer(nmEndp).Redial()
+		p.log.Debugf("NetworkManager dialed on try %d", n)
 		wg.Done()
 	}()
 	wg.Wait()
 
 	p.powerd = powerd.New(powerdEndp, p.log)
 	p.polld = polld.New(polldEndp, p.log)
-	p.urfkill = urfkill.New(urEndp, urWLANKillswitchEndp, p.log)
+	p.nm = networkmanager.New(nmEndp, p.log)
 
 	// busy sleep loop to workaround go's timer/sleep
 	// not accounting for time when the system is suspended
@@ -154,7 +150,7 @@
 	if p.log == nil {
 		return ErrUnconfigured
 	}
-	if p.powerd == nil || p.polld == nil || p.urfkill == nil {
+	if p.powerd == nil || p.polld == nil || p.nm == nil {
 		return ErrNotStarted
 	}
 	wakeupCh, err := p.powerd.WatchWakeups()
@@ -165,19 +161,13 @@
 	if err != nil {
 		return err
 	}
-	flightMode := p.urfkill.IsFlightMode()
-	wlanKillswitchState := p.urfkill.GetWLANKillswitchState()
-	flightModeCh, _, err := p.urfkill.WatchFlightMode()
-	if err != nil {
-		return err
-	}
-	wlanKillswitchStateCh, _, err := p.urfkill.WatchWLANKillswitchState()
-	if err != nil {
-		return err
-	}
-
+	nmState := p.nm.GetState()
+	nmStateCh, _, err := p.nm.WatchState()
+	if err != nil {
+		return err
+	}
 	filteredWakeUpCh := make(chan bool)
-	go p.control(wakeupCh, filteredWakeUpCh, flightMode, flightModeCh, wlanKillswitchState, wlanKillswitchStateCh)
+	go p.control(wakeupCh, filteredWakeUpCh, nmState, nmStateCh)
 	go p.run(filteredWakeUpCh, doneCh)
 	return nil
 }
@@ -195,9 +185,10 @@
 	return t, cookie, err
 }
 
-func (p *poller) control(wakeupCh <-chan bool, filteredWakeUpCh chan<- bool, flightMode bool, flightModeCh <-chan bool, wlanKillswitchState urfkill.KillswitchState, wlanKillswitchStateCh <-chan urfkill.KillswitchState) {
-	wirelessEnabled := wlanKillswitchState == urfkill.KillswitchStateUnblocked
-	dontPoll := flightMode && !wirelessEnabled
+func (p *poller) control(wakeupCh <-chan bool, filteredWakeUpCh chan<- bool, nmState networkmanager.State, nmStateCh <-chan networkmanager.State) {
+	connected := nmState == networkmanager.ConnectedGlobal
+	dontPoll := !connected
+	p.log.Debugf("nmState: %v, networkmanager.ConnectedGlobal: %v", nmState, networkmanager.ConnectedGlobal)
 	var t time.Time
 	cookie := ""
 	holdsWakeLock := false
@@ -234,12 +225,11 @@
 					filteredWakeUpCh <- true
 				}
 			}
-		case flightMode = <-flightModeCh:
-		case wlanKillswitchState = <-wlanKillswitchStateCh:
-			wirelessEnabled = wlanKillswitchState == urfkill.KillswitchStateUnblocked
+		case nmState = <-nmStateCh:
+			connected = nmState == networkmanager.ConnectedGlobal
 		}
-		newDontPoll := flightMode && !wirelessEnabled
-		p.log.Debugf("control: flightMode:%v wirelessEnabled:%v prevDontPoll:%v dontPoll:%v wakeupReq:%v holdsWakeLock:%v", flightMode, wirelessEnabled, dontPoll, newDontPoll, !t.IsZero(), holdsWakeLock)
+		newDontPoll := !connected
+		p.log.Debugf("control: nmState:%v prevDontPoll:%v dontPoll:%v wakeupReq:%v holdsWakeLock:%v", nmState, dontPoll, newDontPoll, !t.IsZero(), holdsWakeLock)
 		if newDontPoll != dontPoll {
 			if dontPoll = newDontPoll; dontPoll {
 				if !t.IsZero() {

=== modified file 'poller/poller_test.go'
--- poller/poller_test.go	2015-04-29 15:22:03 +0000
+++ poller/poller_test.go	2015-09-28 08:25:01 +0000
@@ -21,7 +21,7 @@
 
 	. "launchpad.net/gocheck"
 
-	"launchpad.net/ubuntu-push/bus/urfkill"
+	"launchpad.net/ubuntu-push/bus/networkmanager"
 	"launchpad.net/ubuntu-push/client/session"
 	helpers "launchpad.net/ubuntu-push/testing"
 )
@@ -92,8 +92,8 @@
 }
 
 const (
-	wlanOn  = urfkill.KillswitchStateUnblocked
-	wlanOff = urfkill.KillswitchStateSoftBlocked
+	connectedGlobal    = networkmanager.ConnectedGlobal
+	disconnectedGlobal = networkmanager.Disconnected
 )
 
 func (s *PrSuite) TestStep(c *C) {
@@ -117,7 +117,7 @@
 	ch := make(chan string)
 	// now, run
 	filteredWakeUpCh := make(chan bool)
-	go p.control(wakeupCh, filteredWakeUpCh, false, nil, wlanOn, nil)
+	go p.control(wakeupCh, filteredWakeUpCh, connectedGlobal, nil)
 	go func() { ch <- p.step(filteredWakeUpCh, doneCh, "old cookie") }()
 	select {
 	case s := <-ch:
@@ -143,9 +143,8 @@
 	wakeUpCh := make(chan bool)
 	filteredWakeUpCh := make(chan bool)
 	s.myd.watchWakeCh = make(chan bool, 1)
-	flightModeCh := make(chan bool)
-	wlanKillswitchStateCh := make(chan urfkill.KillswitchState)
-	go p.control(wakeUpCh, filteredWakeUpCh, false, flightModeCh, wlanOn, wlanKillswitchStateCh)
+	nmStateCh := make(chan networkmanager.State)
+	go p.control(wakeUpCh, filteredWakeUpCh, connectedGlobal, nmStateCh)
 
 	// works
 	err := p.requestWakeup()
@@ -161,19 +160,17 @@
 	wakeUpCh <- true
 	<-filteredWakeUpCh
 
-	// flight mode
-	flightModeCh <- true
-	wlanKillswitchStateCh <- wlanOff
+	nmStateCh <- disconnectedGlobal
 	err = p.requestWakeup()
 	c.Assert(err, IsNil)
 	c.Check(s.myd.watchWakeCh, HasLen, 0)
 
-	// wireless on
-	wlanKillswitchStateCh <- wlanOn
+	// connected
+	nmStateCh <- connectedGlobal
 	c.Check(<-s.myd.watchWakeCh, Equals, true)
 
-	// wireless off
-	wlanKillswitchStateCh <- wlanOff
+	// disconnected
+	nmStateCh <- disconnectedGlobal
 	// pending wakeup was cleared
 	c.Check(<-s.myd.watchWakeCh, Equals, false)