You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2017/04/05 01:44:32 UTC

[3/6] incubator-mynewt-newtmgr git commit: nmxact - Clean up BLE session code.

nmxact - Clean up BLE session code.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/commit/7dfbeb0d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/tree/7dfbeb0d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/diff/7dfbeb0d

Branch: refs/heads/master
Commit: 7dfbeb0db8c16148290522e862fd9630dca25e5c
Parents: f1e662f
Author: Christopher Collins <cc...@apache.org>
Authored: Tue Apr 4 15:07:18 2017 -0700
Committer: Christopher Collins <cc...@apache.org>
Committed: Tue Apr 4 18:10:19 2017 -0700

----------------------------------------------------------------------
 nmxact/nmble/ble_fsm.go        | 61 +++++++++++++++++++++----------------
 nmxact/nmble/ble_oic_sesn.go   | 29 ++++++++++++------
 nmxact/nmble/ble_plain_sesn.go | 29 ++++++++++++------
 3 files changed, 72 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/blob/7dfbeb0d/nmxact/nmble/ble_fsm.go
----------------------------------------------------------------------
diff --git a/nmxact/nmble/ble_fsm.go b/nmxact/nmble/ble_fsm.go
index f9492c8..dc94ca9 100644
--- a/nmxact/nmble/ble_fsm.go
+++ b/nmxact/nmble/ble_fsm.go
@@ -59,14 +59,13 @@ type BleFsmParams struct {
 type BleFsm struct {
 	params BleFsmParams
 
-	peerDev    *BleDev
-	connHandle int
-	nmpSvc     *BleSvc
-	nmpReqChr  *BleChr
-	nmpRspChr  *BleChr
-	attMtu     int
-	connChan   chan error
-
+	peerDev         *BleDev
+	connHandle      int
+	nmpSvc          *BleSvc
+	nmpReqChr       *BleChr
+	nmpRspChr       *BleChr
+	attMtu          int
+	connChan        chan error
 	mtx             sync.Mutex
 	lastStateChange time.Time
 
@@ -106,12 +105,16 @@ func (bf *BleFsm) getState() BleSesnState {
 	return bf.state
 }
 
+func (bf *BleFsm) setStateNoLock(toState BleSesnState) {
+	bf.state = toState
+	bf.lastStateChange = time.Now()
+}
+
 func (bf *BleFsm) setState(toState BleSesnState) {
 	bf.mtx.Lock()
 	defer bf.mtx.Unlock()
 
-	bf.state = toState
-	bf.lastStateChange = time.Now()
+	bf.setStateNoLock(toState)
 }
 
 func (bf *BleFsm) transitionState(fromState BleSesnState,
@@ -127,7 +130,7 @@ func (bf *BleFsm) transitionState(fromState BleSesnState,
 			toState, fromState)
 	}
 
-	bf.state = toState
+	bf.setStateNoLock(toState)
 	return nil
 }
 
@@ -135,14 +138,13 @@ func (bf *BleFsm) addBleListener(base BleMsgBase) (*BleListener, error) {
 	bl := NewBleListener()
 
 	bf.mtx.Lock()
-	bf.bls[bl] = struct{}{}
-	bf.mtx.Unlock()
+	defer bf.mtx.Unlock()
 
 	if err := bf.params.Bx.Bd.AddListener(base, bl); err != nil {
-		delete(bf.bls, bl)
 		return nil, err
 	}
 
+	bf.bls[bl] = struct{}{}
 	return bl, nil
 }
 
@@ -162,11 +164,12 @@ func (bf *BleFsm) addBleSeqListener(seq BleSeq) (*BleListener, error) {
 }
 
 func (bf *BleFsm) removeBleListener(base BleMsgBase) {
+	bf.mtx.Lock()
+	defer bf.mtx.Unlock()
+
 	bl := bf.params.Bx.Bd.RemoveListener(base)
 	if bl != nil {
-		bf.mtx.Lock()
 		delete(bf.bls, bl)
-		bf.mtx.Unlock()
 	}
 }
 
@@ -200,8 +203,8 @@ func (bf *BleFsm) action(
 	return nil
 }
 
-func (bf *BleFsm) calcDisconnectType() BleFsmDisconnectType {
-	switch bf.getState() {
+func calcDisconnectType(state BleSesnState) BleFsmDisconnectType {
+	switch state {
 	case SESN_STATE_EXCHANGING_MTU:
 		return FSM_DISCONNECT_TYPE_IMMEDIATE_TIMEOUT
 
@@ -220,19 +223,23 @@ func (bf *BleFsm) onDisconnect(err error) {
 	log.Debugf(err.Error())
 
 	bf.mtx.Lock()
-	bls := make([]*BleListener, 0, len(bf.bls))
-	for bl, _ := range bf.bls {
-		bls = append(bls, bl)
-	}
-	bf.mtx.Unlock()
 
 	// Remember some fields before we clear them.
-	dt := bf.calcDisconnectType()
+	dt := calcDisconnectType(bf.state)
 	peer := *bf.peerDev
 
-	bf.setState(SESN_STATE_UNCONNECTED)
+	bf.setStateNoLock(SESN_STATE_UNCONNECTED)
 	bf.peerDev = nil
 
+	// Make a copy of all the listeners so we don't have to keep the mutex
+	// locked while we send error signals to them.
+	bls := make([]*BleListener, 0, len(bf.bls))
+	for bl, _ := range bf.bls {
+		bls = append(bls, bl)
+	}
+
+	bf.mtx.Unlock()
+
 	for _, bl := range bls {
 		bl.ErrChan <- err
 	}
@@ -253,7 +260,7 @@ func (bf *BleFsm) connectListen(seq BleSeq) error {
 		for {
 			select {
 			case err := <-bl.ErrChan:
-				// Transport reported error.  Assume all connections have
+				// Transport reported error.  Assume the connection has
 				// dropped.
 				bf.onDisconnect(err)
 				return
@@ -453,7 +460,7 @@ func (bf *BleFsm) terminateSetState() error {
 		return fmt.Errorf(
 			"BLE terminate failed; session already being closed")
 	default:
-		bf.state = SESN_STATE_TERMINATING
+		bf.setStateNoLock(SESN_STATE_TERMINATING)
 	}
 
 	return nil

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/blob/7dfbeb0d/nmxact/nmble/ble_oic_sesn.go
----------------------------------------------------------------------
diff --git a/nmxact/nmble/ble_oic_sesn.go b/nmxact/nmble/ble_oic_sesn.go
index 5fa9291..e5139be 100644
--- a/nmxact/nmble/ble_oic_sesn.go
+++ b/nmxact/nmble/ble_oic_sesn.go
@@ -23,7 +23,7 @@ type BleOicSesn struct {
 	onCloseCb    sesn.BleOnCloseFn
 
 	closeChan chan error
-	mx        sync.Mutex
+	mtx       sync.Mutex
 }
 
 func NewBleOicSesn(bx *BleXport, cfg sesn.SesnCfg) *BleOicSesn {
@@ -67,18 +67,22 @@ func NewBleOicSesn(bx *BleXport, cfg sesn.SesnCfg) *BleOicSesn {
 }
 
 func (bos *BleOicSesn) addNmpListener(seq uint8) (*nmp.NmpListener, error) {
-	nl := nmp.NewNmpListener()
-	bos.nls[nl] = struct{}{}
+	bos.mtx.Lock()
+	defer bos.mtx.Unlock()
 
+	nl := nmp.NewNmpListener()
 	if err := bos.od.AddListener(seq, nl); err != nil {
-		delete(bos.nls, nl)
 		return nil, err
 	}
 
+	bos.nls[nl] = struct{}{}
 	return nl, nil
 }
 
 func (bos *BleOicSesn) removeNmpListener(seq uint8) {
+	bos.mtx.Lock()
+	defer bos.mtx.Unlock()
+
 	listener := bos.od.RemoveListener(seq)
 	if listener != nil {
 		delete(bos.nls, listener)
@@ -87,8 +91,8 @@ func (bos *BleOicSesn) removeNmpListener(seq uint8) {
 
 // Returns true if a new channel was assigned.
 func (bos *BleOicSesn) setCloseChan() error {
-	bos.mx.Lock()
-	defer bos.mx.Unlock()
+	bos.mtx.Lock()
+	defer bos.mtx.Unlock()
 
 	if bos.closeChan != nil {
 		return fmt.Errorf("Multiple listeners waiting for session to close")
@@ -99,8 +103,8 @@ func (bos *BleOicSesn) setCloseChan() error {
 }
 
 func (bos *BleOicSesn) clearCloseChan() {
-	bos.mx.Lock()
-	defer bos.mx.Unlock()
+	bos.mtx.Lock()
+	defer bos.mtx.Unlock()
 
 	bos.closeChan = nil
 }
@@ -126,7 +130,7 @@ func (bos *BleOicSesn) blockUntilClosed(timeout time.Duration) error {
 		return nil
 	}
 
-	// Block until close completes or timeout.
+	// Block until close completes or times out.
 	return bos.listenForClose(timeout)
 }
 
@@ -173,7 +177,7 @@ func (bos *BleOicSesn) Close() error {
 		return nil
 	}
 
-	// Block until close completes or timeout.
+	// Block until close completes or times out.
 	return bos.listenForClose(bos.closeTimeout)
 }
 
@@ -185,9 +189,12 @@ func (bos *BleOicSesn) onRxNmp(data []byte) {
 	bos.od.Dispatch(data)
 }
 
+// Called by the FSM when a blehostd disconnect event is received.
 func (bos *BleOicSesn) onDisconnect(dt BleFsmDisconnectType, peer BleDev,
 	err error) {
 
+	bos.mtx.Lock()
+
 	for nl, _ := range bos.nls {
 		nl.ErrChan <- err
 	}
@@ -197,6 +204,8 @@ func (bos *BleOicSesn) onDisconnect(dt BleFsmDisconnectType, peer BleDev,
 		bos.closeChan <- err
 	}
 
+	bos.mtx.Unlock()
+
 	// Only execute client's disconnect callback if the disconnect was
 	// unsolicited and the session was fully open.
 	if dt == FSM_DISCONNECT_TYPE_OPENED && bos.onCloseCb != nil {

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newtmgr/blob/7dfbeb0d/nmxact/nmble/ble_plain_sesn.go
----------------------------------------------------------------------
diff --git a/nmxact/nmble/ble_plain_sesn.go b/nmxact/nmble/ble_plain_sesn.go
index 15419a9..803eb45 100644
--- a/nmxact/nmble/ble_plain_sesn.go
+++ b/nmxact/nmble/ble_plain_sesn.go
@@ -22,7 +22,7 @@ type BlePlainSesn struct {
 	onCloseCb    sesn.BleOnCloseFn
 
 	closeChan chan error
-	mx        sync.Mutex
+	mtx       sync.Mutex
 }
 
 func NewBlePlainSesn(bx *BleXport, cfg sesn.SesnCfg) *BlePlainSesn {
@@ -61,18 +61,22 @@ func NewBlePlainSesn(bx *BleXport, cfg sesn.SesnCfg) *BlePlainSesn {
 }
 
 func (bps *BlePlainSesn) addNmpListener(seq uint8) (*nmp.NmpListener, error) {
-	nl := nmp.NewNmpListener()
-	bps.nls[nl] = struct{}{}
+	bps.mtx.Lock()
+	defer bps.mtx.Unlock()
 
+	nl := nmp.NewNmpListener()
 	if err := bps.nd.AddListener(seq, nl); err != nil {
-		delete(bps.nls, nl)
 		return nil, err
 	}
 
+	bps.nls[nl] = struct{}{}
 	return nl, nil
 }
 
 func (bps *BlePlainSesn) removeNmpListener(seq uint8) {
+	bps.mtx.Lock()
+	defer bps.mtx.Unlock()
+
 	listener := bps.nd.RemoveListener(seq)
 	if listener != nil {
 		delete(bps.nls, listener)
@@ -80,8 +84,8 @@ func (bps *BlePlainSesn) removeNmpListener(seq uint8) {
 }
 
 func (bps *BlePlainSesn) setCloseChan() error {
-	bps.mx.Lock()
-	defer bps.mx.Unlock()
+	bps.mtx.Lock()
+	defer bps.mtx.Unlock()
 
 	if bps.closeChan != nil {
 		return fmt.Errorf("Multiple listeners waiting for session to close")
@@ -92,8 +96,8 @@ func (bps *BlePlainSesn) setCloseChan() error {
 }
 
 func (bps *BlePlainSesn) clearCloseChan() {
-	bps.mx.Lock()
-	defer bps.mx.Unlock()
+	bps.mtx.Lock()
+	defer bps.mtx.Unlock()
 
 	bps.closeChan = nil
 }
@@ -119,7 +123,7 @@ func (bps *BlePlainSesn) blockUntilClosed(timeout time.Duration) error {
 		return nil
 	}
 
-	// Block until close completes or timeout.
+	// Block until close completes or times out.
 	return bps.listenForClose(timeout)
 }
 
@@ -166,7 +170,7 @@ func (bps *BlePlainSesn) Close() error {
 		return nil
 	}
 
-	// Block until close completes or timeout.
+	// Block until close completes or times out.
 	return bps.listenForClose(bps.closeTimeout)
 }
 
@@ -178,9 +182,12 @@ func (bps *BlePlainSesn) onRxNmp(data []byte) {
 	bps.nd.Dispatch(data)
 }
 
+// Called by the FSM when a blehostd disconnect event is received.
 func (bps *BlePlainSesn) onDisconnect(dt BleFsmDisconnectType, peer BleDev,
 	err error) {
 
+	bps.mtx.Lock()
+
 	for nl, _ := range bps.nls {
 		nl.ErrChan <- err
 	}
@@ -190,6 +197,8 @@ func (bps *BlePlainSesn) onDisconnect(dt BleFsmDisconnectType, peer BleDev,
 		bps.closeChan <- err
 	}
 
+	bps.mtx.Unlock()
+
 	// Only execute client's disconnect callback if the disconnect was
 	// unsolicited and the session was fully open.
 	if dt == FSM_DISCONNECT_TYPE_OPENED && bps.onCloseCb != nil {