[mynewt-newtmgr] 02/04: nmxact - Fix duplicate notification listener bug.

[cc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ccollins pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-newtmgr.git

commit 73025e88977ee642185dbb1cbea283a2af0db0f2
Author: Christopher Collins <cc...@apache.org>
AuthorDate: Thu Aug 17 17:50:05 2017 -0700

    nmxact - Fix duplicate notification listener bug.
---
 nmxact/bledefs/bledefs.go | 10 ++++++++++
 nmxact/nmble/ble_sesn.go  |  9 +++------
 nmxact/nmble/conn.go      | 40 ++++++++++++++++++++--------------------
 nmxact/nmble/profile.go   |  4 ++++
 4 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/nmxact/bledefs/bledefs.go b/nmxact/bledefs/bledefs.go
index e6bb5b9..5538681 100644
--- a/nmxact/bledefs/bledefs.go
+++ b/nmxact/bledefs/bledefs.go
@@ -824,6 +824,16 @@ func (b *BleChrId) String() string {
 	return fmt.Sprintf("s=%s c=%s", b.SvcUuid.String(), b.ChrUuid.String())
 }
 
+func CompareChrIds(a BleChrId, b BleChrId) int {
+	if rc := CompareUuids(a.SvcUuid, b.SvcUuid); rc != 0 {
+		return rc
+	}
+	if rc := CompareUuids(a.ChrUuid, b.ChrUuid); rc != 0 {
+		return rc
+	}
+	return 0
+}
+
 type BleMgmtChrs struct {
 	NmpReqChr       *BleChrId
 	NmpRspChr       *BleChrId
diff --git a/nmxact/nmble/ble_sesn.go b/nmxact/nmble/ble_sesn.go
index 3719750..2355c30 100644
--- a/nmxact/nmble/ble_sesn.go
+++ b/nmxact/nmble/ble_sesn.go
@@ -140,7 +140,7 @@ func (s *BleSesn) createNotifyListener(chrId *BleChrId) (
 		return nil, err
 	}
 
-	return s.conn.ListenForNotifications(chr), nil
+	return s.conn.ListenForNotifications(chr)
 }
 
 func (s *BleSesn) notifyListenOnce(chrId *BleChrId,
@@ -175,13 +175,10 @@ func (s *BleSesn) notifyListenOnce(chrId *BleChrId,
 }
 
 func (s *BleSesn) notifyListen() {
-	s.notifyListenOnce(s.mgmtChrs.NmpRspChr, s.txvr.DispatchNmpRsp)
 	s.notifyListenOnce(s.mgmtChrs.ResUnauthRspChr, s.txvr.DispatchCoap)
 	s.notifyListenOnce(s.mgmtChrs.ResSecureRspChr, s.txvr.DispatchCoap)
-
-	// XXX: Don't listen for public resource responses for now; characteristic
-	// may conflict with newtmgr.
-	//s.notifyListenOnce(s.mgmtChrs.ResPublicRspChr, s.txvr.DispatchCoap)
+	s.notifyListenOnce(s.mgmtChrs.ResPublicRspChr, s.txvr.DispatchCoap)
+	s.notifyListenOnce(s.mgmtChrs.NmpRspChr, s.txvr.DispatchNmpRsp)
 }
 
 func (s *BleSesn) openOnce() (bool, error) {
diff --git a/nmxact/nmble/conn.go b/nmxact/nmble/conn.go
index 5912d3d..40acaa0 100644
--- a/nmxact/nmble/conn.go
+++ b/nmxact/nmble/conn.go
@@ -46,7 +46,7 @@ type Conn struct {
 	// Terminates all go routines.  Gets set to null after disconnect.
 	stopChan chan struct{}
 
-	notifyMap map[*Characteristic][](*NotifyListener)
+	notifyMap map[*Characteristic]*NotifyListener
 
 	// Protects:
 	// * connHandle
@@ -64,7 +64,7 @@ func NewConn(bx *BleXport) *Conn {
 		attMtu:         BLE_ATT_MTU_DFLT,
 		disconnectChan: make(chan error, 1),
 		stopChan:       make(chan struct{}),
-		notifyMap:      map[*Characteristic][](*NotifyListener){},
+		notifyMap:      map[*Characteristic]*NotifyListener{},
 	}
 }
 
@@ -88,12 +88,10 @@ func (c *Conn) initiateShutdown() bool {
 func (c *Conn) abortNotifyListeners(err error) {
 	// No need to lock mutex; this should only be called after all go routines
 	// have terminated.
-	for _, nls := range c.notifyMap {
-		for _, nl := range nls {
-			nl.ErrChan <- err
-			close(nl.NotifyChan)
-			close(nl.ErrChan)
-		}
+	for _, nl := range c.notifyMap {
+		nl.ErrChan <- err
+		close(nl.NotifyChan)
+		close(nl.ErrChan)
 	}
 }
 
@@ -201,19 +199,16 @@ func (c *Conn) rxNotify(msg *BleNotifyRxEvt) {
 		return
 	}
 
-	nls := c.notifyMap[chr]
-	if nls == nil {
+	nl := c.notifyMap[chr]
+	if nl == nil {
 		return
 	}
 
-	n := Notification{
+	nl.NotifyChan <- Notification{
 		Chr:        chr,
 		Data:       msg.Data.Bytes,
 		Indication: msg.Indication,
 	}
-	for _, nl := range nls {
-		nl.NotifyChan <- n
-	}
 }
 
 // Listens for incoming notifications and indications.
@@ -662,17 +657,22 @@ func (c *Conn) Subscribe(chr *Characteristic) error {
 	return c.WriteHandle(dsc.Handle, payload, "subscribe")
 }
 
-func (c *Conn) ListenForNotifications(chr *Characteristic) *NotifyListener {
+func (c *Conn) ListenForNotifications(chr *Characteristic) (
+	*NotifyListener, error) {
+
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 
-	nl := NewNotifyListener()
-	slice := c.notifyMap[chr]
+	if _, ok := c.notifyMap[chr]; ok {
+		return nil, fmt.Errorf(
+			"Already listening for notifications on characteristic %s",
+			chr.String())
+	}
 
-	slice = append(slice, nl)
-	c.notifyMap[chr] = slice
+	nl := NewNotifyListener()
+	c.notifyMap[chr] = nl
 
-	return nl
+	return nl, nil
 }
 
 func (c *Conn) InitiateSecurity() error {
diff --git a/nmxact/nmble/profile.go b/nmxact/nmble/profile.go
index 0e98740..9db78e9 100644
--- a/nmxact/nmble/profile.go
+++ b/nmxact/nmble/profile.go
@@ -31,6 +31,10 @@ type Profile struct {
 	attrs map[uint16]*Characteristic
 }
 
+func (c *Characteristic) String() string {
+	return c.Uuid.String()
+}
+
 func (c *Characteristic) SubscribeType() BleChrFlags {
 	if c.Properties&BLE_GATT_F_NOTIFY != 0 {
 		return BLE_GATT_F_NOTIFY

-- 
To stop receiving notification emails like this one, please contact
"commits@mynewt.apache.org" <co...@mynewt.apache.org>.