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 <[email protected]> Authored: Tue Apr 4 15:07:18 2017 -0700 Committer: Christopher Collins <[email protected]> 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 {
