From 45497de08dd9a6e11c84882858e9d15247c38af3 Mon Sep 17 00:00:00 2001 From: Peter Wood Date: Sun, 9 Feb 2020 13:57:41 +0000 Subject: [PATCH] Rework node request to use handle response correctly. Remove all but happy path tests for active endpoints, everything else covered by node_request. --- node_active_endpoints.go | 12 ++-- node_active_endpoints_test.go | 115 +--------------------------------- node_request.go | 24 +++---- node_request_test.go | 42 ++++++++----- 4 files changed, 47 insertions(+), 146 deletions(-) diff --git a/node_active_endpoints.go b/node_active_endpoints.go index 27ade32..fef4bc1 100644 --- a/node_active_endpoints.go +++ b/node_active_endpoints.go @@ -11,14 +11,18 @@ func (z *ZStack) QueryNodeEndpoints(ctx context.Context, networkAddress zigbee.N OfInterestAddress: networkAddress, } - resp := ZdoActiveEpRsp{} - - err := z.nodeRequest(ctx, &request, &ZdoActiveEpReqReply{}, &resp, func(i interface{}) bool { + resp, err := z.nodeRequest(ctx, &request, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, func(i interface{}) bool { msg := i.(*ZdoActiveEpRsp) return msg.OfInterestAddress == networkAddress }) - return resp.ActiveEndpoints, err + castResp, ok := resp.(*ZdoActiveEpRsp) + + if ok { + return castResp.ActiveEndpoints, err + } else { + return nil, err + } } type ZdoActiveEpReq struct { diff --git a/node_active_endpoints_test.go b/node_active_endpoints_test.go index 973a5db..6d8cb76 100644 --- a/node_active_endpoints_test.go +++ b/node_active_endpoints_test.go @@ -12,50 +12,7 @@ import ( ) func Test_QueryNodeEndpoints(t *testing.T) { - t.Run("returns an error if the query fails", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - - unpiMock := unpiTest.NewMockAdapter() - zstack := New(unpiMock) - defer unpiMock.Stop() - - unpiMock.On(SREQ, ZDO, ZdoActiveEpReqID).Return(Frame{ - MessageType: SRSP, - Subsystem: ZDO, - CommandID: ZdoActiveEpReqReplyID, - Payload: []byte{0x01}, - }) - - _, err := zstack.QueryNodeEndpoints(ctx, zigbee.NetworkAddress(0x2000)) - assert.Error(t, err) - assert.Equal(t, ErrorZFailure, err) - - unpiMock.AssertCalls(t) - }) - - t.Run("returns an success on query, but no response is seen", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) - defer cancel() - - unpiMock := unpiTest.NewMockAdapter() - zstack := New(unpiMock) - defer unpiMock.Stop() - - unpiMock.On(SREQ, ZDO, ZdoActiveEpReqID).Return(Frame{ - MessageType: SRSP, - Subsystem: ZDO, - CommandID: ZdoActiveEpReqReplyID, - Payload: []byte{0x00}, - }) - - _, err := zstack.QueryNodeEndpoints(ctx, zigbee.NetworkAddress(0x2000)) - assert.Error(t, err) - - unpiMock.AssertCalls(t) - }) - - t.Run("returns an success on query, response for requested network address is received", func(t *testing.T) { + t.Run("returns an success on query, response for requested network address is received", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() @@ -86,76 +43,6 @@ func Test_QueryNodeEndpoints(t *testing.T) { unpiMock.AssertCalls(t) }) - - t.Run("returns an success on query, response for requested unwanted and then wanted network address is received", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - - unpiMock := unpiTest.NewMockAdapter() - zstack := New(unpiMock) - defer unpiMock.Stop() - - unpiMock.On(SREQ, ZDO, ZdoActiveEpReqID).Return(Frame{ - MessageType: SRSP, - Subsystem: ZDO, - CommandID: ZdoActiveEpReqReplyID, - Payload: []byte{0x00}, - }) - - go func() { - time.Sleep(10 * time.Millisecond) - unpiMock.InjectOutgoing(Frame{ - MessageType: AREQ, - Subsystem: ZDO, - CommandID: ZdoActiveEpRspID, - Payload: []byte{0x00, 0x20, 0x00, 0x00, 0x20, 0x03, 0x01, 0x02, 0x03}, - }) - time.Sleep(10 * time.Millisecond) - unpiMock.InjectOutgoing(Frame{ - MessageType: AREQ, - Subsystem: ZDO, - CommandID: ZdoActiveEpRspID, - Payload: []byte{0x00, 0x20, 0x00, 0x00, 0x40, 0x01, 0x02}, - }) - }() - - endpoints, err := zstack.QueryNodeEndpoints(ctx, zigbee.NetworkAddress(0x4000)) - assert.NoError(t, err) - assert.Equal(t, []byte{0x02}, endpoints) - - unpiMock.AssertCalls(t) - }) - - t.Run("returns an success on query, response from node is an error", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - - unpiMock := unpiTest.NewMockAdapter() - zstack := New(unpiMock) - defer unpiMock.Stop() - - unpiMock.On(SREQ, ZDO, ZdoActiveEpReqID).Return(Frame{ - MessageType: SRSP, - Subsystem: ZDO, - CommandID: ZdoActiveEpReqReplyID, - Payload: []byte{0x00}, - }) - - go func() { - time.Sleep(10 * time.Millisecond) - unpiMock.InjectOutgoing(Frame{ - MessageType: AREQ, - Subsystem: ZDO, - CommandID: ZdoActiveEpRspID, - Payload: []byte{0x00, 0x20, 0x01, 0x00, 0x40, 0x03, 0x01, 0x02, 0x03}, - }) - }() - - _, err := zstack.QueryNodeEndpoints(ctx, zigbee.NetworkAddress(0x4000)) - assert.Error(t, err) - - unpiMock.AssertCalls(t) - }) } func Test_ActiveEndpointMessages(t *testing.T) { diff --git a/node_request.go b/node_request.go index a32001e..9ab3f69 100644 --- a/node_request.go +++ b/node_request.go @@ -12,19 +12,19 @@ func AnyResponse(v interface{}) bool { var ReplyDoesNotReportSuccess = errors.New("reply struct does not support Successor interface") var NodeResponseWasNotSuccess = errors.New("response from node was not success") -func (z *ZStack) nodeRequest(ctx context.Context, request interface{}, reply interface{}, response interface{}, responseFilter func(interface{}) bool) error { +func (z *ZStack) nodeRequest(ctx context.Context, request interface{}, reply interface{}, response interface{}, responseFilter func(interface{}) bool) (interface{}, error) { replySuccessor, replySupportsSuccessor := reply.(Successor) if !replySupportsSuccessor { - return ReplyDoesNotReportSuccess + return nil, ReplyDoesNotReportSuccess } - ch := make(chan bool) + ch := make(chan interface{}) err, stop := z.subscriber.Subscribe(response, func(v interface{}) { if responseFilter(v) { select { - case ch <- true: + case ch <- v: case <-ctx.Done(): } } @@ -32,26 +32,26 @@ func (z *ZStack) nodeRequest(ctx context.Context, request interface{}, reply int defer stop() if err != nil { - return err + return nil, err } if err := z.requestResponder.RequestResponse(ctx, request, reply); err != nil { - return err + return nil, err } if !replySuccessor.WasSuccessful() { - return ErrorZFailure + return nil, ErrorZFailure } select { - case <-ch: - responseSuccessor, responseSupportsSuccessor := response.(Successor) + case v := <-ch: + responseSuccessor, responseSupportsSuccessor := v.(Successor) if responseSupportsSuccessor && !responseSuccessor.WasSuccessful() { - return NodeResponseWasNotSuccess + return v, NodeResponseWasNotSuccess } - return nil + return v, nil case <-ctx.Done(): - return errors.New("context expired while waiting for response from node") + return nil, errors.New("context expired while waiting for response from node") } } diff --git a/node_request_test.go b/node_request_test.go index b3a46c7..a8f9231 100644 --- a/node_request_test.go +++ b/node_request_test.go @@ -21,9 +21,10 @@ func Test_NodeRequest(t *testing.T) { type NotSuccessful struct {} - err := zstack.nodeRequest(ctx, &NotSuccessful{}, &NotSuccessful{}, &NotSuccessful{}, AnyResponse) + resp, err := zstack.nodeRequest(ctx, &NotSuccessful{}, &NotSuccessful{}, &NotSuccessful{}, AnyResponse) assert.Error(t, err) + assert.Nil(t, resp) assert.Equal(t, ReplyDoesNotReportSuccess, err) }) @@ -42,8 +43,10 @@ func Test_NodeRequest(t *testing.T) { Payload: []byte{0x01}, }) - err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{}, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, AnyResponse) + resp, err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{}, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, AnyResponse) + assert.Error(t, err) + assert.Nil(t, resp) assert.Equal(t, ErrorZFailure, err) unpiMock.AssertCalls(t) @@ -74,10 +77,12 @@ func Test_NodeRequest(t *testing.T) { }) }() - response := &ZdoActiveEpRsp{} - err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, response, AnyResponse) + resp, err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, AnyResponse) + castResp, ok := resp.(*ZdoActiveEpRsp) + assert.NoError(t, err) - assert.Equal(t, []byte{0x01, 0x02, 0x03}, response.ActiveEndpoints) + assert.True(t, ok) + assert.Equal(t, []byte{0x01, 0x02, 0x03}, castResp.ActiveEndpoints) unpiMock.AssertCalls(t) }) @@ -114,13 +119,15 @@ func Test_NodeRequest(t *testing.T) { }) }() - response := &ZdoActiveEpRsp{} - err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, response, func(i interface{}) bool { + resp, err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, func(i interface{}) bool { response := i.(*ZdoActiveEpRsp) return response.OfInterestAddress == 0x4000 }) + castResp, ok := resp.(*ZdoActiveEpRsp) + assert.NoError(t, err) - assert.Equal(t, []byte{0x02}, response.ActiveEndpoints) + assert.True(t, ok) + assert.Equal(t, []byte{0x02}, castResp.ActiveEndpoints) unpiMock.AssertCalls(t) }) @@ -156,13 +163,14 @@ func Test_NodeRequest(t *testing.T) { }) }() - response := &ZdoActiveEpRsp{} - err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, response, func(i interface{}) bool { - response := i.(*ZdoActiveEpRsp) - return response.OfInterestAddress == 0x4000 + resp, err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, func(i interface{}) bool { + return i.(*ZdoActiveEpRsp).OfInterestAddress == 0x4000 }) + castResp, ok := resp.(*ZdoActiveEpRsp) + assert.NoError(t, err) - assert.Equal(t, []byte{0x02}, response.ActiveEndpoints) + assert.True(t, ok) + assert.Equal(t, []byte{0x02}, castResp.ActiveEndpoints) unpiMock.AssertCalls(t) }) @@ -192,11 +200,13 @@ func Test_NodeRequest(t *testing.T) { }) }() - response := &ZdoActiveEpRsp{} - err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, response, AnyResponse) + resp, err := zstack.nodeRequest(ctx, &ZdoActiveEpReq{DestinationAddress:0x4000, OfInterestAddress:0x4000}, &ZdoActiveEpReqReply{}, &ZdoActiveEpRsp{}, AnyResponse) + castResp, ok := resp.(*ZdoActiveEpRsp) + assert.Error(t, err) + assert.True(t, ok) assert.Equal(t, NodeResponseWasNotSuccess, err) - assert.Equal(t, []byte{0x01, 0x02, 0x03}, response.ActiveEndpoints) + assert.Equal(t, []byte{0x01, 0x02, 0x03}, castResp.ActiveEndpoints) unpiMock.AssertCalls(t) })