Merge ~codersquid/snappy-hwe-snaps/+git/wifi-connect:log-settings into ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-connect:master

Proposed by Sheila Miguez
Status: Merged
Approved by: Roberto Mier Escandon
Approved revision: 406b82e64d9031bc871ded4bbfa7771f8e6ec59e
Merged at revision: bbd4f274c402c09b7bf6f0a6e68412d01ac05b7b
Proposed branch: ~codersquid/snappy-hwe-snaps/+git/wifi-connect:log-settings
Merge into: ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-connect:master
Diff against target: 891 lines (+125/-142)
16 files modified
avahi/avahi.go (+3/-5)
cmd/main.go (+6/-2)
daemon/daemon.go (+16/-15)
hooks/configure.go (+5/-3)
netman/dbus.go (+21/-20)
netman/dbus_test.go (+1/-1)
server/fmt.go (+5/-6)
server/fmt_test.go (+3/-31)
server/handlers.go (+9/-10)
server/launcher.go (+2/-2)
server/launcher_test.go (+5/-1)
service/service.go (+15/-10)
spread_tests/main/ap/task.yaml (+2/-2)
utils/fmt.go (+17/-20)
utils/fmt_test.go (+12/-12)
utils/utils.go (+3/-2)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Approve
Sheila Miguez (community) Approve
Roberto Mier Escandon (community) Approve
Kyle Nitzsche (community) Approve
Konrad Zapałowicz (community) code Approve
Review via email: mp+325739@code.launchpad.net

Description of the change

This change configures logging with a prefix and with some helpful information.

The prefix string "== wifi-connect" will not need to be added by hand to calls to the logger anymore.

The log lines will also have helpful information, <prefix> YYYY/MM/DD HH:TT:SS <filename>:<linenumber>

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Roberto Mier Escandon (rmescandon) wrote :

I guess this is valid per process, right. Regardless of such a generic name, that file is the entry point for command "wifi-connect", but there is also a service running at the same time at service/service.go. I this log config needed to be executed in both cases?

review: Needs Information
Revision history for this message
Sheila Miguez (codersquid) wrote :

d'oh I fixed it.

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Konrad Zapałowicz (kzapalowicz) wrote :

ack

review: Approve (code)
Revision history for this message
Roberto Mier Escandon (rmescandon) :
review: Approve
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

I have a BIG request.

Currently almost all of our log messages simply use fmt.Println("== ..."). As a result our logs are visually unified and easy to read.

For example:

Mar 14 20:32:31 pi3dev snap[1262]: == wifi-connect: daemon STARTING
Jun 14 15:57:47 pi3dev snap[1262]: == wifi-connect: wifi-ap is UP but has no SSIDS
Jun 14 15:57:55 pi3dev snap[1262]: == wifi-connect: entering MANAGEMENT mode
Jun 14 15:57:57 pi3dev snap[1262]: == wifi-connect: SSID(s) obtained
Jun 14 15:57:58 pi3dev snap[1262]: == wifi-connect: starting wifi-ap
Jun 14 16:11:02 pi3dev snap[1262]: == wifi-connect: SSID(s) obtained
Jun 14 16:11:04 pi3dev snap[1262]: == wifi-connect/RefreshHandler: starting wifi-ap
Jun 14 16:17:50 pi3dev snap[1262]: == wifi-connect/handler: Connecting to astro_garden_2
Jun 14 16:17:58 pi3dev snap[1262]: == wifi-connect: entering OPERATIONAL mode
Jun 14 16:17:58 pi3dev snap[1262]: 2017/06/14 16:17:58 == wifi-connect/server: HTTP Server closing - accept tcp [::]:8080: use of closed network connection
Jun 14 16:18:19 pi3dev snap[1262]: == wifi-connect: entering MANAGEMENT mode
Jun 14 16:18:21 pi3dev snap[1262]: == wifi-connect: SSID(s) obtained
Jun 14 16:18:22 pi3dev snap[1262]: == wifi-connect: starting wifi-ap

The inconsistent one is the HTTP server message, which has a second timestamp due to using log.

I'd like consistency here. Can you change everything to using the log pkg so that we don't postpone work and we don't have to live with increasing inconsistency in the mean time?

review: Needs Fixing
Revision history for this message
Sheila Miguez (codersquid) wrote :

On Fri, Jun 16, 2017 at 8:42 AM, Kyle Nitzsche <email address hidden>
wrote:

> I'd like consistency here. Can you change everything to using the log pkg
> so that we don't postpone work and we don't have to live with increasing
> inconsistency in the mean time?

I understand, but I want to get the log settings in a minimal changeset so
that I don't spend time tidying all of the places where I think we should
be using log versus fmt and also clearing out the manual prefixes. That
would take away time from doing the other things.

I can leave this MR for later if you prefer a larger change.

--
<email address hidden>

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

I'd prefer to get the whole thing done at once instead of increasing inconsistency and postponing catch-up work.

Revision history for this message
Sheila Miguez (codersquid) wrote :

Ok, I think I've caught every fmt call and place where we've used the prefix.

I also changed the log flags to remove the date and time.

I switched to using the long file name for logs, so that we get fully qualified pathnames and line numbers. I think we should let this suffice for package names. If we have consensus, I'll remove the utils/fmt.go and its callers.

I can't get spread to work, so this is not fully tested. I need help with that.

Revision history for this message
Sheila Miguez (codersquid) wrote :

I left fmt along in the cmd package. I don't think we'll want to prefix things that show up in the command line. Yes?

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

On 06/16/2017 03:51 PM, Sheila Miguez wrote:
> I left fmt along in the cmd package. I don't think we'll want to prefix things that show up in the command line. Yes?
>
right, and thx

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Roberto Mier Escandon (rmescandon) wrote :

LGTM.
Other than that, though not affected by recent changes (so, no conflicts), this branch is not aligned with master one. If you need to add some extra stuff, take care of merging/rebasing master first.

review: Approve
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

LGTM

review: Approve
Revision history for this message
Sheila Miguez (codersquid) wrote :

Spread test failed due to the grep for 'enter' versus 'Enter'. I reverted back to 'enter'. I also reverted the grammar change I made to another log line.

Revision history for this message
Roberto Mier Escandon (rmescandon) wrote :

I'm afraid now there are conflicts with master, as one (huge) MR has just landed :S

review: Needs Fixing
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sheila Miguez (codersquid) wrote :

Not sure what to do about the failing jenkins test for server. The tests pass in my environment. https://pastebin.canonical.com/191305/

Revision history for this message
Roberto Mier Escandon (rmescandon) wrote :

Here locally, it is also passing.
Maybe it is a temporary failure in jenkins. Seems that it is not taking initial state for some tests (first time it happens). Try to push nothing new to force jenkins be triggered again:

git commit --amend
git push -f

If again the failure happens, I'll try to make some changes in those tests to be passed (if possible)

Revision history for this message
Sheila Miguez (codersquid) wrote :

I'm pushing an empty change like you suggest. Let's see if this works!

review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Roberto Mier Escandon (rmescandon) wrote :

lgtm

review: Approve
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Hey Skay: Just checking: lines 421 and 422 of the diff. Does the new line (422) drop the "/handler" output from the log message?

Same for a couple other places.

Revision history for this message
Sheila Miguez (codersquid) wrote :

On Wed, Jun 21, 2017 at 8:15 AM, Kyle Nitzsche <email address hidden>
wrote:

> Hey Skay: Just checking: lines 421 and 422 of the diff. Does the new line
> (422) drop the "/handler" output from the log message?
>
> Same for a couple other places.
>

Yes, but the new log lines will include the path of the file that generated
the log. This means the 'handler' string will show up. Is that sufficient?

--
<email address hidden>

Revision history for this message
Sheila Miguez (codersquid) wrote :

in fact, at some point I'd like to remove the custom fmt code since the
functions in it are used to include the package name. but the package name
will be in the path, which means we don't need the extra code.

--
<email address hidden>

Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

I see. I often used the log message to indicate the func name (albeit inconsistently). This is fine though. carry on :)

Revision history for this message
Sheila Miguez (codersquid) wrote :

After chatting, we decided to skip the test that is failing in jenkins for a few reasons. 1) it is not properly a unit. This type of thing belongs in spread tests. 2) we will be changing to use a new server when doing https.

review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sheila Miguez (codersquid) wrote :

hopefully spread works this time.

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sheila Miguez (codersquid) wrote :

once more

Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Sheila Miguez (codersquid) wrote :

The automerge failed, and I'm not sure of the details.

https://jenkins.canonical.com/system-enablement/job/lp-snappy-hwe-automerger/83542/console

ERROR: Failed to merge https://code.launchpad.net/~codersquid/snappy-hwe-snaps/+git/wifi-connect/+merge/325739 (You tried to access a resource that you don't have the server-side permission to see.)
Build step 'Execute shell' marked build as failure
Finished: FAILURE

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/avahi/avahi.go b/avahi/avahi.go
2index 435d2ff..49165e1 100644
3--- a/avahi/avahi.go
4+++ b/avahi/avahi.go
5@@ -28,8 +28,6 @@ import (
6 "github.com/presotto/go-mdns-sd"
7 )
8
9-var logger *log.Logger
10-
11 type mdnsScanner interface {
12 ScanInterfaces() (string, error)
13 }
14@@ -56,10 +54,10 @@ func InitMDNS() error {
15 // these or snapweb has to be restarted
16 var err error
17 hostname := getHostname()
18- log.Println("Registering hostname:", hostname)
19+ log.Printf("Registering hostname: %s", hostname)
20 _mdns, err = newMDNS(hostname, "", "", false, 0)
21 if err != nil {
22- log.Println("Cannot create mDNS instance:", err)
23+ log.Printf("Cannot create mDNS instance: %v", err)
24 return fmt.Errorf("Cannot create mDNS instance: %s", err.Error())
25 }
26 // poll to update published IP addresses for this mDNS name; ideally
27@@ -86,7 +84,7 @@ var osHostname = os.Hostname
28 func getHostname() (hostname string) {
29 hostname, err := osHostname()
30 if err != nil {
31- logger.Println("Cannot obtain hostname, using default:", err)
32+ log.Printf("Cannot obtain hostname, using default: %v", err)
33 return hostnameDefault
34 }
35 hostname = strings.Split(hostname, ".")[0]
36diff --git a/cmd/main.go b/cmd/main.go
37index e168570..2eebb48 100644
38--- a/cmd/main.go
39+++ b/cmd/main.go
40@@ -20,6 +20,7 @@ package main
41 import (
42 "bufio"
43 "fmt"
44+ "log"
45 "os"
46 "os/signal"
47 "strings"
48@@ -72,8 +73,11 @@ func waitForCtrlC() {
49
50 func main() {
51
52+ log.SetFlags(log.Lshortfile)
53+ log.SetPrefix("== wifi-connect: ")
54+
55 if len(os.Args) < 2 {
56- fmt.Println("== wifi-connect/cmd Error: no command arguments provided")
57+ fmt.Println("Error: no command arguments provided")
58 return
59 }
60 args := os.Args[1:]
61@@ -96,7 +100,7 @@ func main() {
62 fmt.Println(err)
63 return
64 }
65- fmt.Println("Entering MANUAL Mode. Wifi-connect has stopped managing state. Use 'start' to restore normal operations")
66+ fmt.Println("entering MANUAL Mode. Wifi-connect has stopped managing state. Use 'start' to restore normal operations")
67 case "start":
68 if !checkSudo() {
69 return
70diff --git a/daemon/daemon.go b/daemon/daemon.go
71index a888969..944dbbf 100644
72--- a/daemon/daemon.go
73+++ b/daemon/daemon.go
74@@ -21,6 +21,7 @@ import (
75 "encoding/json"
76 "fmt"
77 "io/ioutil"
78+ "log"
79 "os"
80 "path/filepath"
81
82@@ -128,13 +129,13 @@ func (c *Client) ManualMode() bool {
83 if _, err := os.Stat(manualFlagPath); os.IsNotExist(err) {
84 if state == MANUAL {
85 c.SetState(STARTING)
86- fmt.Println("== wifi-connect: entering STARTING mode")
87+ log.Print("entering STARTING mode")
88 }
89 return false
90 }
91 if state != MANUAL {
92 c.SetState(MANUAL)
93- fmt.Println("== wifi-connect: entering MANUAL mode")
94+ log.Print("entering MANUAL mode")
95 }
96 return true
97 }
98@@ -150,7 +151,7 @@ func (c *Client) IsApUpWithoutSSIDs(cw *wifiap.Client) bool {
99 }
100 ssids, _ := utils.ReadSsidsFile()
101 if len(ssids) < 1 {
102- fmt.Println("== wifi-connect: wifi-ap is UP but has no SSIDS")
103+ log.Print("wifi-ap is UP but has no SSIDS")
104 return true // ap is up with no ssids
105 }
106 return false
107@@ -162,7 +163,7 @@ func (c *Client) ManagementServerUp() {
108 if server.Current != server.Management && server.State == server.Stopped {
109 err = server.StartManagementServer()
110 if err != nil {
111- fmt.Println("== wifi-connect: Error start Mamagement portal:", err)
112+ log.Printf("Error start Mamagement portal: %v", err)
113 }
114 // init mDNS
115 avahi.InitMDNS()
116@@ -175,7 +176,7 @@ func (c *Client) ManagementServerDown() {
117 if server.Current == server.Management && (server.State == server.Running || server.State == server.Starting) {
118 err = server.ShutdownManagementServer()
119 if err != nil {
120- fmt.Println("== wifi-connect: Error stopping the Management portal:", err)
121+ log.Printf("Error stopping the Management portal: %v", err)
122 }
123 //remove flag fie so daemon resumes normal control
124 utils.RemoveFlagFile(os.Getenv("SNAP_COMMON") + "/startingApConnect")
125@@ -188,7 +189,7 @@ func (c *Client) OperationalServerUp() {
126 if server.Current != server.Operational && server.State == server.Stopped {
127 err = server.StartOperationalServer()
128 if err != nil {
129- fmt.Println("== wifi-connect: Error starting the Operational portal:", err)
130+ log.Printf("Error starting the Operational portal: %v", err)
131 }
132 // init mDNS
133 avahi.InitMDNS()
134@@ -200,7 +201,7 @@ func (c *Client) OperationalServerDown() {
135 if server.Current == server.Operational && (server.State == server.Running || server.State == server.Starting) {
136 err = server.ShutdownOperationalServer()
137 if err != nil {
138- fmt.Println("== wifi-connect: Error stopping Operational portal:", err)
139+ log.Printf("Error stopping Operational portal: %v", err)
140 }
141 }
142 }
143@@ -211,7 +212,7 @@ func LoadPreConfig() (*PreConfig, error) {
144 config := &PreConfig{}
145 content, err := ioutil.ReadFile(PreConfigFile)
146 if err == nil {
147- fmt.Println("== wifi-connect/daemon: preconfiguration file found")
148+ log.Print("preconfiguration file found")
149 }
150 err = json.Unmarshal(content, config)
151 if err != nil {
152@@ -225,18 +226,18 @@ func LoadPreConfig() (*PreConfig, error) {
153 // indicates whether a pre-config file exists.
154 func (c *Client) SetDefaults(cw wifiap.Operations, config *PreConfig) error {
155 if err != nil {
156- fmt.Println("== wifi-connect/daemon/SetDefaults: preconfig unmarshall errorr:", err)
157+ log.Printf("SetDefaults: preconfig unmarshall errorr: %v", err)
158 }
159 ap, errShow := cw.Show()
160 if errShow != nil {
161- fmt.Println("== wifi-connect/daemon/SetDefaults: wifi-ap.Show err:", errShow)
162+ log.Printf("SetDefaults: wifi-ap.Show err: %v", errShow)
163 }
164 if ap["wifi.security-passphrase"] != config.Passphrase {
165 if len(config.Passphrase) > 0 {
166 err = cw.SetPassphrase(config.Passphrase)
167- fmt.Println("== wifi-connect/SetDefaults wifi-ap passphrase being set")
168+ log.Print("SetDefaults wifi-ap passphrase being set")
169 if err != nil {
170- fmt.Println("== wifi-connect/daemon/SetDefaults: passphrase err:", err)
171+ log.Printf("SetDefaults: passphrase err: %v", err)
172 return err
173 }
174 }
175@@ -245,15 +246,15 @@ func (c *Client) SetDefaults(cw wifiap.Operations, config *PreConfig) error {
176 fmt.Println("== wifi-connect/SetDefaults portal password being set")
177 _, err = utils.HashIt(config.Password)
178 if err != nil {
179- fmt.Println("== wifi-connect/daemon/SetDefaults: password err:", err)
180+ log.Printf("SetDefaults: password err: %v", err)
181 return err
182 }
183 }
184 if config.NoOperational {
185- fmt.Println("== wifi-connect/SetDefaults: operational portal is now disabled")
186+ log.Print("SetDefaults: operational portal is now disabled")
187 }
188 if config.NoResetCreds {
189- fmt.Println("== wifi-connect/SetDefaults: reset creds requirement is now disabled")
190+ log.Print("SetDefaults: reset creds requirement is now disabled")
191 }
192 return nil
193 }
194diff --git a/hooks/configure.go b/hooks/configure.go
195index 601a93a..0477d4c 100644
196--- a/hooks/configure.go
197+++ b/hooks/configure.go
198@@ -69,7 +69,7 @@ func (c *Client) snapGetStr(key string, target *string) {
199 return
200 }
201 if len(val) == 0 {
202- log.Printf("== wifi-connect/configure error: key %s exists but has zero length", key)
203+ log.Printf("configure error: key %s exists but has zero length", key)
204 return
205 }
206 *target = val
207@@ -82,7 +82,7 @@ func (c *Client) snapGetBool(key string, target *bool) {
208 return
209 }
210 if len(val) == 0 {
211- log.Printf("== wifi-connect/configure error: key %s exists but has zero length", key)
212+ log.Printf("configure error: key %s exists but has zero length", key)
213 return
214 }
215
216@@ -94,6 +94,8 @@ func (c *Client) snapGetBool(key string, target *bool) {
217 }
218
219 func main() {
220+ log.SetFlags(log.Llongfile)
221+ log.SetPrefix("== wifi-connect: ")
222 client := GetClient()
223 preConfig := &daemon.PreConfig{}
224 client.snapGetStr("wifi.security-passphrase", &preConfig.Passphrase)
225@@ -105,7 +107,7 @@ func main() {
226 if errJM == nil {
227 errWJ := ioutil.WriteFile(daemon.PreConfigFile, b, 0644)
228 if errWJ != nil {
229- log.Print("== wifi-connect/configure error:", errWJ)
230+ log.Print("configure error:", errWJ)
231 }
232 }
233 }
234diff --git a/netman/dbus.go b/netman/dbus.go
235index 13f536b..2db7107 100644
236--- a/netman/dbus.go
237+++ b/netman/dbus.go
238@@ -20,6 +20,7 @@ package netman
239 import (
240 "errors"
241 "fmt"
242+ "log"
243 "os"
244 "strings"
245 "time"
246@@ -92,7 +93,7 @@ func (c *Client) GetDevices() []string {
247 var devices []string
248 err := c.dbusClient.BusObj.Call("org.freedesktop.NetworkManager.GetAllDevices", 0).Store(&devices)
249 if err != nil {
250- fmt.Println("== wifi-connect: Error getting devices:", err)
251+ log.Printf("Error getting devices: %v", err)
252 }
253 return devices
254 }
255@@ -106,7 +107,7 @@ func (c *Client) GetWifiDevices(devices []string) []string {
256 setObject(c, "org.freedesktop.NetworkManager", objPath)
257 deviceType, err2 := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.DeviceType")
258 if err2 != nil {
259- fmt.Println("== wifi-connect: Error getting wifi devices:", err2)
260+ log.Printf("Error getting wifi devices: %v", err2)
261 continue
262 }
263 var wifiType uint32
264@@ -132,7 +133,7 @@ func (c *Client) GetAccessPoints(devices []string, ap2device map[string]string)
265 setObject(c, "org.freedesktop.NetworkManager", objPath)
266 err := c.dbusClient.BusObj.Call("org.freedesktop.NetworkManager.Device.Wireless.GetAllAccessPoints", 0).Store(&aps)
267 if err != nil {
268- fmt.Println("== wifi-connect: Error getting accesspoints:", err)
269+ log.Printf("Error getting accesspoints: %v", err)
270 continue
271 }
272 if len(aps) == 0 {
273@@ -161,7 +162,7 @@ func (c *Client) getSsids(APs []string, ssid2ap map[string]string) []SSID {
274 setObject(c, "org.freedesktop.NetworkManager", objPath)
275 ssid, err := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.AccessPoint.Ssid")
276 if err != nil {
277- fmt.Println("== wifi-connect: Error getting accesspoint's ssids:", err)
278+ log.Printf("Error getting accesspoint's ssids: %v", err)
279 continue
280 }
281 type B []byte
282@@ -219,7 +220,7 @@ func (c *Client) ConnectAp(ssid string, p string, ap2device map[string]string, s
283 func getSystemBus() *dbus.Conn {
284 conn, err := dbus.SystemBus()
285 if err != nil {
286- fmt.Fprintln(os.Stderr, "== wifi-connect: Error: Failed to connect to system bus:", err)
287+ log.Printf("Error: Failed to connect to system bus: %v", err)
288 panic(1)
289 }
290 return conn
291@@ -244,12 +245,12 @@ func (c *Client) Connected(devices []string) bool {
292 setObject(c, "org.freedesktop.NetworkManager", objPath)
293 dType, err := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.DeviceType")
294 if err != nil {
295- fmt.Println("== wifi-connect: Error getting device type:", err)
296+ log.Printf("Error getting device type: %v", err)
297 continue
298 }
299 state, err2 := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.State")
300 if err2 != nil {
301- fmt.Println("== wifi-connect: Error getting device state:", err2)
302+ log.Printf("Error getting device state: %v", err2)
303 continue
304 }
305 // only handle eth and wifi device type
306@@ -271,7 +272,7 @@ func (c *Client) ConnectedWifi(wifiDevices []string) bool {
307 setObject(c, "org.freedesktop.NetworkManager", objPath)
308 state, err := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.State")
309 if err != nil {
310- fmt.Println("== wifi-connect: Error getting device state:", err)
311+ log.Printf("Error getting device state: %v", err)
312 continue
313 }
314 if dbus.Variant.Value(state) == uint32(100) {
315@@ -302,7 +303,7 @@ func (c *Client) SetIfaceManaged(iface string, state bool, devices []string) str
316 setObject(c, "org.freedesktop.NetworkManager", objPath)
317 intface, err2 := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.Interface")
318 if err2 != nil {
319- fmt.Printf("== wifi-connect: Error in SetIfaceManaged() geting interface: %v\n", err2)
320+ log.Printf("Error in SetIfaceManaged() geting interface: %v", err2)
321 return ""
322 }
323 if iface != intface.Value().(string) {
324@@ -310,7 +311,7 @@ func (c *Client) SetIfaceManaged(iface string, state bool, devices []string) str
325 }
326 managed, err := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.Managed")
327 if err != nil {
328- fmt.Printf("== wifi-connect: Error in SetIfaceManaged() fetching device managed: %v\n", err)
329+ log.Printf("Error in SetIfaceManaged() fetching device managed: %v", err)
330 return ""
331 }
332 switch state {
333@@ -360,12 +361,12 @@ func (c *Client) WifisManaged(wifiDevices []string) (map[string]string, error) {
334 setObject(c, "org.freedesktop.NetworkManager", objPath)
335 managed, err := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.Managed")
336 if err != nil {
337- fmt.Printf("== wifi-connect: Error in WifisManaged() getting device managed : %v\n", err)
338+ log.Printf("Error in WifisManaged() getting device managed: %v", err)
339 return ifaces, err
340 }
341 iface, err2 := c.dbusClient.BusObj.GetProperty("org.freedesktop.NetworkManager.Device.Interface")
342 if err2 != nil {
343- fmt.Printf("== wifi-connect: Error in WifisManaged() getting device interface: %v\n", err)
344+ log.Printf("Error in WifisManaged() getting device interface: %v", err)
345 return ifaces, err2
346 }
347 if managed.Value().(bool) == true {
348@@ -380,11 +381,11 @@ func (c *Client) WifisManaged(wifiDevices []string) (map[string]string, error) {
349 func (c *Client) Unmanage() error {
350 ifaces, err := c.WifisManaged(c.GetWifiDevices(c.GetDevices()))
351 if err != nil {
352- return fmt.Errorf("== wifi-connect: Error getting managed wifis: %v", err)
353+ return fmt.Errorf("Error getting managed wifis: %v", err)
354 }
355 if _, ok := ifaces["wlan0"]; ok {
356 if c.SetIfaceManaged("wlan0", false, c.GetWifiDevices(c.GetDevices())) == "" {
357- return fmt.Errorf("== wifi-connect: No interface was set to unmanaged")
358+ return fmt.Errorf("No interface was set to unmanaged")
359 }
360 }
361 return nil
362@@ -393,7 +394,7 @@ func (c *Client) Unmanage() error {
363 // Manage sets wlan0 to not managed by network manager
364 func (c *Client) Manage() error {
365 if c.SetIfaceManaged("wlan0", true, c.GetWifiDevices(c.GetDevices())) == "" {
366- return fmt.Errorf("== wifi-connect: No interface was set to managed")
367+ return fmt.Errorf("No interface was set to managed")
368 }
369 return nil
370 }
371@@ -406,14 +407,14 @@ func (c *Client) ScanAndWriteSsidsToFile(filepath string) bool {
372 if _, err = os.Stat(filepath); os.IsNotExist(err) {
373 file, err = os.Create(filepath)
374 if err != nil {
375- fmt.Printf("Error touching ssids file: %v\n", err)
376+ log.Printf("Error touching ssids file: %v", err)
377 return false
378 }
379 }
380
381 file, err = os.OpenFile(filepath, os.O_RDWR, 0644)
382 if err != nil {
383- fmt.Printf("Error opening ssids file: %v\n", err)
384+ log.Printf("Error opening ssids file: %v", err)
385 return false
386 }
387
388@@ -438,12 +439,12 @@ func (c *Client) scanSsids(writer io.Writer) bool {
389 out = out[:len(out)-1]
390 _, err := writer.Write([]byte(out))
391 if err != nil {
392- fmt.Printf("== wifi-connect: Error writing SSID(s): %v\n ", err)
393+ log.Printf("Error writing SSID(s): %v ", err)
394 } else {
395- fmt.Println("== wifi-connect: SSID(s) obtained")
396+ log.Print("SSID(s) obtained")
397 return true
398 }
399 }
400- fmt.Println("== wifi-connect: No SSID found")
401+ log.Print("No SSID found")
402 return false
403 }
404diff --git a/netman/dbus_test.go b/netman/dbus_test.go
405index 4b9e3e6..c27a2f6 100644
406--- a/netman/dbus_test.go
407+++ b/netman/dbus_test.go
408@@ -240,7 +240,7 @@ func TestConnectedWifi(t *testing.T) {
409 }
410 }
411
412-func TestiDiscconnectWifi(t *testing.T) {
413+func TestDiscconnectWifi(t *testing.T) {
414 client := NewClient(&mockObj{})
415 res := client.DisconnectWifi([]string{})
416 if res != 0 {
417diff --git a/server/fmt.go b/server/fmt.go
418index a614bd3..62fb3ea 100644
419--- a/server/fmt.go
420+++ b/server/fmt.go
421@@ -1,16 +1,15 @@
422 package server
423
424-import (
425- "launchpad.net/wifi-connect/utils"
426-)
427+import "launchpad.net/wifi-connect/utils"
428
429-// Errorf returns a new instance implementing error interface taken a formatted string and
430-// params as input
431+// Errorf implements the Error interface. It returns an Error with the "server"
432+// package name automatically inserted as a prefix.
433 func Errorf(format string, a ...interface{}) error {
434 return utils.PkgErrorf("server", format, a...)
435 }
436
437-// Sprintf returns a formatted string with project prefix
438+// Sprintf returns a formatted string with the "server"
439+// package name automatically inserted as a prefix.
440 func Sprintf(format string, a ...interface{}) string {
441 return utils.PkgSprintf("server", format, a...)
442 }
443diff --git a/server/fmt_test.go b/server/fmt_test.go
444index bec4a03..b1f9ed9 100644
445--- a/server/fmt_test.go
446+++ b/server/fmt_test.go
447@@ -1,10 +1,6 @@
448 package server
449
450-import (
451- "testing"
452-
453- "launchpad.net/wifi-connect/utils"
454-)
455+import "testing"
456
457 const (
458 pkg = "server"
459@@ -14,7 +10,7 @@ func TestErrorFormat(t *testing.T) {
460
461 msg := "Whatever message"
462 e := Errorf(msg)
463- expected := utils.FmtPrefix + "/" + pkg + ": " + msg
464+ expected := pkg + ": " + msg
465
466 if e.Error() != expected {
467 t.Errorf("Error is not well formatted, expected %v but got %v", expected, e.Error())
468@@ -26,33 +22,9 @@ func TestErrorfFormat(t *testing.T) {
469 detail := "message"
470 e := Errorf(format, detail)
471
472- expected := utils.FmtPrefix + "/" + pkg + ": Whatever message"
473+ expected := pkg + ": Whatever message"
474
475 if e.Error() != expected {
476 t.Errorf("Error is not well formatted, expected %s but got %s", expected, e.Error())
477 }
478 }
479-
480-func TestSprintFormat(t *testing.T) {
481-
482- msg := "Whatever message"
483- e := Sprintf(msg)
484- expected := utils.FmtPrefix + "/" + pkg + ": " + msg
485-
486- if e != expected {
487- t.Errorf("String is not well formatted, expected %s but got %s", expected, e)
488- }
489-}
490-
491-func TestSprintfFormat(t *testing.T) {
492-
493- format := "Whatever %v"
494- detail := "message"
495- e := Sprintf(format, detail)
496-
497- expected := utils.FmtPrefix + "/" + pkg + ": Whatever message"
498-
499- if e != expected {
500- t.Errorf("String is not well formatted, expected %s but got %s", expected, e)
501- }
502-}
503diff --git a/server/handlers.go b/server/handlers.go
504index 99bed8f..f28c2d8 100644
505--- a/server/handlers.go
506+++ b/server/handlers.go
507@@ -61,14 +61,14 @@ func execTemplate(w http.ResponseWriter, templatePath string, data Data) {
508 templateAbsPath := filepath.Join(ResourcesPath, templatePath)
509 t, err := template.ParseFiles(templateAbsPath)
510 if err != nil {
511- fmt.Printf("== wifi-connect/handler: Error loading the template at %v : %v\n", templatePath, err)
512+ log.Printf("Error loading the template at %v: %v", templatePath, err)
513 http.Error(w, err.Error(), http.StatusInternalServerError)
514 return
515 }
516
517 err = t.Execute(w, data)
518 if err != nil {
519- fmt.Printf("== wifi-connect/handler: Error executing the template at %v : %v\n", templatePath, err)
520+ log.Printf("Error executing the template at %v: %v", templatePath, err)
521 http.Error(w, err.Error(), http.StatusInternalServerError)
522 return
523 }
524@@ -79,7 +79,7 @@ func ManagementHandler(w http.ResponseWriter, r *http.Request) {
525 // daemon stores current available ssids in a file
526 ssids, err := utils.ReadSsidsFile()
527 if err != nil {
528- fmt.Printf("== wifi-connect/handler: Error reading SSIDs file: %v\n", err)
529+ log.Printf("Error reading SSIDs file: %v", err)
530 http.Error(w, err.Error(), http.StatusInternalServerError)
531 return
532 }
533@@ -103,7 +103,7 @@ func ConnectHandler(w http.ResponseWriter, r *http.Request) {
534
535 ssids := r.Form["ssid"]
536 if len(ssids) == 0 {
537- log.Print(Sprintf("SSID not available"))
538+ log.Print("SSID not available")
539 return
540 }
541 ssid := ssids[0]
542@@ -112,11 +112,11 @@ func ConnectHandler(w http.ResponseWriter, r *http.Request) {
543 execTemplate(w, connectingTemplatePath, data)
544
545 go func() {
546- log.Print(Sprintf("Connecting to %v\n", ssid))
547+ log.Printf("Connecting to %v", ssid)
548
549 err := wifiapClient.Disable()
550 if err != nil {
551- log.Print(Sprintf("Error disabling AP: %v\n", err))
552+ log.Printf("Error disabling AP: %v", err)
553 return
554 }
555
556@@ -127,7 +127,7 @@ func ConnectHandler(w http.ResponseWriter, r *http.Request) {
557 err = netmanClient.ConnectAp(ssid, pwd, ap2device, ssid2ap)
558 //TODO signal user in portal on failure to connect
559 if err != nil {
560- log.Print(Sprintf("Failed connecting to %v.\n", ssid))
561+ log.Printf("Failed connecting to %v.", ssid)
562 return
563 }
564
565@@ -136,7 +136,6 @@ func ConnectHandler(w http.ResponseWriter, r *http.Request) {
566 waitPath := os.Getenv("SNAP_COMMON") + "/startingApConnect"
567 utils.RemoveFlagFile(waitPath)
568 }()
569-
570 }
571
572 type disconnectData struct {
573@@ -159,7 +158,7 @@ func HashItHandler(w http.ResponseWriter, r *http.Request) {
574 hashMe := r.Form["Hash"]
575 hashed, errH := utils.MatchingHash(hashMe[0])
576 if errH != nil {
577- fmt.Println("== wifi-connect/HashitHandler: error hashing:", errH)
578+ log.Printf("HashItHandler: error hashing: %v", errH)
579 return
580 }
581 res := &hashResponse{}
582@@ -167,7 +166,7 @@ func HashItHandler(w http.ResponseWriter, r *http.Request) {
583 res.Err = "no error"
584 b, err := json.Marshal(res)
585 if err != nil {
586- fmt.Println("== wifi-connect/HashItHandler: error mashaling json")
587+ log.Printf("HashItHandler: error marshaling json")
588 return
589 }
590 w.Write(b)
591diff --git a/server/launcher.go b/server/launcher.go
592index e292084..7317df0 100644
593--- a/server/launcher.go
594+++ b/server/launcher.go
595@@ -110,7 +110,7 @@ func listenAndServe(addr string, handler http.Handler) error {
596 }
597
598 if retries == 0 {
599- log.Print(Sprintf("Server could not be started"))
600+ log.Print("Server could not be started")
601 return
602 }
603
604@@ -122,7 +122,7 @@ func listenAndServe(addr string, handler http.Handler) error {
605 if listener != nil {
606 err := srv.Serve(tcpKeepAliveListener{listener.(*net.TCPListener)})
607 if err != nil {
608- log.Printf(Sprintf("HTTP Server closing - %v", err))
609+ log.Printf("HTTP Server closing - %v", err)
610 }
611 // notify server real stop
612 done <- true
613diff --git a/server/launcher_test.go b/server/launcher_test.go
614index c6d9a0e..c377c79 100644
615--- a/server/launcher_test.go
616+++ b/server/launcher_test.go
617@@ -45,7 +45,11 @@ func TestLaunchAndStop(t *testing.T) {
618 }
619 }
620
621-func TestStates(t *testing.T) {
622+// This test is being skipped because of a problem with jenkins where it fails.
623+// We cannot reproduce the failure outside of jenkins. Additionally, we are
624+// changing the http server and will not track state in the same way in the future.
625+// These tests will change.
626+func SkipTestStates(t *testing.T) {
627
628 WaitForState(Stopped)
629
630diff --git a/service/service.go b/service/service.go
631index c988f55..568b3dd 100644
632--- a/service/service.go
633+++ b/service/service.go
634@@ -19,6 +19,7 @@ package main
635
636 import (
637 "fmt"
638+ "log"
639 "os"
640 "time"
641
642@@ -29,17 +30,21 @@ import (
643 )
644
645 func main() {
646+
647+ log.SetFlags(log.Lshortfile)
648+ log.SetPrefix("== wifi-connect: ")
649+
650 c := netman.DefaultClient()
651 cw := wifiap.DefaultClient()
652 client := daemon.GetClient()
653
654 config, err := daemon.LoadPreConfig()
655 if err != nil {
656- fmt.Println("== wifi-connect/daemon: preconfiguration error:", err)
657+ log.Printf("daemon: preconfiguration error: %v", err)
658 }
659 err = client.SetDefaults(cw, config)
660 if err != nil {
661- fmt.Println("== wifi-connect/daemon: SetDetaults error:", err)
662+ log.Printf("daemon: SetDetaults error: %v", err)
663 }
664 first := true
665 client.SetWaitFlagPath(os.Getenv("SNAP_COMMON") + "/startingApConnect")
666@@ -50,7 +55,7 @@ func main() {
667
668 for {
669 if first {
670- fmt.Println("== wifi-connect: daemon STARTING")
671+ log.Print("daemon STARTING")
672 client.SetPreviousState(daemon.STARTING)
673 client.SetState(daemon.STARTING)
674 first = false
675@@ -96,7 +101,7 @@ func main() {
676 if c.ConnectedWifi(c.GetWifiDevices(c.GetDevices())) {
677 client.SetState(daemon.OPERATING)
678 if client.GetPreviousState() != daemon.OPERATING {
679- fmt.Println("== wifi-connect: entering OPERATIONAL mode")
680+ log.Print("entering OPERATIONAL mode")
681 }
682 if client.GetPreviousState() == daemon.MANAGING {
683 client.ManagementServerDown()
684@@ -109,20 +114,20 @@ func main() {
685
686 client.SetState(daemon.MANAGING)
687 if client.GetPreviousState() != daemon.MANAGING {
688- fmt.Println("== wifi-connect: entering MANAGEMENT mode")
689+ log.Print("entering MANAGEMENT mode")
690 }
691
692 // if wlan0 managed, set Unmanaged so that we can bring up wifi-ap
693 // properly
694 if err := c.Unmanage(); err != nil {
695- fmt.Println(err)
696+ log.Print(err)
697 continue
698 }
699
700 //wifi-ap UP?
701 wifiUp, err := cw.Enabled()
702 if err != nil {
703- fmt.Println("== wifi-connect: Error checking wifi-ap.Enabled():", err)
704+ log.Printf("Error checking wifi-ap.Enabled(): %v", err)
705 continue // try again since no better course of action
706 }
707
708@@ -134,12 +139,12 @@ func main() {
709 continue
710 }
711 if !found {
712- fmt.Println("== wifi-connect: Looping.")
713+ log.Print("No SSIDs found. Continuing to scan for SSIDS...")
714 continue
715 }
716- fmt.Println("== wifi-connect: starting wifi-ap")
717+ log.Printf("starting wifi-ap")
718 if err := cw.Enable(); err != nil {
719- fmt.Println(err)
720+ log.Print(err)
721 continue
722 }
723 if client.GetPreviousState() == daemon.OPERATING {
724diff --git a/spread_tests/main/ap/task.yaml b/spread_tests/main/ap/task.yaml
725index b281cd7..f48e181 100644
726--- a/spread_tests/main/ap/task.yaml
727+++ b/spread_tests/main/ap/task.yaml
728@@ -44,7 +44,7 @@ execute: |
729 # This is a known issue, see comments here https://github.com/CanonicalLtd/UCWifiConnect/issues/38
730 sleep 40
731 wifi-connect stop
732- until journalctl | grep '== wifi-connect: entering MANUAL mode' ; do
733+ until journalctl | grep 'entering MANUAL mode' ; do
734 sleep 5
735 wifi-connect stop
736 done
737@@ -148,4 +148,4 @@ execute: |
738 done
739 test $i -lt $MAX_ITERATIONS
740
741- nmcli d | grep 'wlan1.*connected'
742\ No newline at end of file
743+ nmcli d | grep 'wlan1.*connected'
744diff --git a/utils/fmt.go b/utils/fmt.go
745index 9408703..0caa47e 100644
746--- a/utils/fmt.go
747+++ b/utils/fmt.go
748@@ -1,32 +1,29 @@
749 package utils
750
751-import (
752- "fmt"
753-)
754+import "fmt"
755
756-const (
757- // FmtPrefix to be included in all log traces
758- FmtPrefix = "== wifi-connect"
759-)
760-
761-// Errorf returns a new instance implementing error interface taken a formatted string and
762-// params as input
763-func Errorf(format string, a ...interface{}) error {
764- return fmt.Errorf(fmt.Sprintf("%v: %v", FmtPrefix, format), a...)
765+// PkgError returns a new instance implementing the Error interface that is intended
766+// to accept the package name as an input. Callers can wrap this to create Error methods
767+// for their packages.
768+func PkgError(pkg string, a ...interface{}) error {
769+ return fmt.Errorf("%v: %v", pkg, a)
770 }
771
772-// PkgErrorf returns a new instance implementing error interface taken a formatted string and
773-// params as input
774+// PkgErrorf returns a new instance implementing the Error interface that is intended
775+// to accept the package name and a format string as inputs. Callers can wrap this to
776+// create Errorf methods for their packages.
777 func PkgErrorf(pkg string, format string, a ...interface{}) error {
778- return fmt.Errorf(fmt.Sprintf("%v/%v: %v", FmtPrefix, pkg, format), a...)
779+ return fmt.Errorf(fmt.Sprintf("%v: %v", pkg, format), a...)
780 }
781
782-// Sprintf returns a formatted string with project prefix
783-func Sprintf(format string, a ...interface{}) string {
784- return fmt.Sprintf(fmt.Sprintf("%v: %v", FmtPrefix, format), a...)
785+// PkgSprint returns a formatted string with a project prefix. Callers can wrap this to
786+// create Sprintf methods for their packages.
787+func PkgSprint(pkg string, a ...interface{}) string {
788+ return fmt.Sprintf("%v: %v", pkg, a)
789 }
790
791-// PkgSprintf returns a formatted string with project prefix
792+// PkgSprintf returns a formatted string with a project prefix. Callers can wrap this to
793+// create Sprintf methods for their packages.
794 func PkgSprintf(pkg string, format string, a ...interface{}) string {
795- return fmt.Sprintf(fmt.Sprintf("%v/%v: %v", FmtPrefix, pkg, format), a...)
796+ return fmt.Sprintf(fmt.Sprintf("%v: %v", pkg, format), a...)
797 }
798diff --git a/utils/fmt_test.go b/utils/fmt_test.go
799index 118b87a..55d94d5 100644
800--- a/utils/fmt_test.go
801+++ b/utils/fmt_test.go
802@@ -4,47 +4,47 @@ import (
803 "testing"
804 )
805
806-func TestErrorFormat(t *testing.T) {
807+func TestPkgErrorFormat(t *testing.T) {
808
809 msg := "Whatever message"
810- e := Errorf(msg)
811- expected := FmtPrefix + ": " + msg
812+ e := PkgErrorf("package", msg)
813+ expected := "package: " + msg
814
815 if e.Error() != expected {
816 t.Errorf("Error is not well formatted, expected %v but got %v", expected, e.Error())
817 }
818 }
819-func TestErrorfFormat(t *testing.T) {
820+func TestPkgErrorfFormat(t *testing.T) {
821
822 format := "Whatever %v"
823 detail := "message"
824- e := Errorf(format, detail)
825+ e := PkgErrorf("package", format, detail)
826
827- expected := FmtPrefix + ": Whatever message"
828+ expected := "package: Whatever message"
829
830 if e.Error() != expected {
831 t.Errorf("Error is not well formatted, expected %s but got %s", expected, e.Error())
832 }
833 }
834
835-func TestSprintFormat(t *testing.T) {
836+func TestPkgSprintFormat(t *testing.T) {
837
838 msg := "Whatever message"
839- e := Sprintf(msg)
840- expected := FmtPrefix + ": " + msg
841+ e := PkgSprintf("package", msg)
842+ expected := "package: " + msg
843
844 if e != expected {
845 t.Errorf("String is not well formatted, expected %s but got %s", expected, e)
846 }
847 }
848
849-func TestSprintfFormat(t *testing.T) {
850+func TestPkgSprintfFormat(t *testing.T) {
851
852 format := "Whatever %v"
853 detail := "message"
854- e := Sprintf(format, detail)
855+ e := PkgSprintf("package", format, detail)
856
857- expected := FmtPrefix + ": Whatever message"
858+ expected := "package: Whatever message"
859
860 if e != expected {
861 t.Errorf("String is not well formatted, expected %s but got %s", expected, e)
862diff --git a/utils/utils.go b/utils/utils.go
863index 384a2db..94850c0 100644
864--- a/utils/utils.go
865+++ b/utils/utils.go
866@@ -23,6 +23,7 @@ import (
867 "fmt"
868 "io"
869 "io/ioutil"
870+ "log"
871 "os"
872 "path/filepath"
873 "sort"
874@@ -48,7 +49,7 @@ func HashIt(s string) ([]byte, error) {
875 }
876 errW := ioutil.WriteFile(HashFile, b, 0644)
877 if errW != nil {
878- fmt.Println("== wifi-connect/HashIt write Error.", err)
879+ log.Printf("HashIt write Error. %v", err)
880 return b, errW
881 }
882 return b, nil
883@@ -59,7 +60,7 @@ func HashIt(s string) ([]byte, error) {
884 func MatchingHash(pword string) (bool, error) {
885 savedHash, err := ioutil.ReadFile(HashFile)
886 if err != nil {
887- fmt.Println("== wifi-connect/matchingHash read Error.", err)
888+ log.Printf("matchingHash read Error. %v", err)
889 return false, err
890 }
891 if bcrypt.CompareHashAndPassword(savedHash, []byte(pword)) == nil {

Subscribers

People subscribed via source and target branches