重构之 SimpliciTi的SMPL_Ping()

辛昕

重构之 SimpliciTi的SMPL_Ping() [复制链接]

重构这本书，和这个话题，偶在这版块里都提了很多次了，虽然，嗯，那啥，都没啥动静，不要紧，咱继续~~~

嗯，其实这本书我已经放下很久了，看了很久的 软件测试艺术 以及刚打算重新看一遍的 C嵌入式测试驱动开发 那本书。
然而，我始终不知道该怎么展开 测试。

最近，遇到一个挺麻烦的问题。
嗯，我一直都在弄CC2530，这是俺的工作。
我用SimpliciTi这个Ti私家的协议栈。普通的收发嘛，是木有问题了。

不过在用它提供的扩展API，比如SMPL_Ping()，今天的主角时，遇到了不少麻烦。

其实在此之前，我也不是没有想到过怎么直接看源码，去看懂这个函数。
但粗粗看了看，我发现，这个函数涉及了一系列底层的操，而这个底层的操作又是一个很复杂的API
嗯，没错，就是SMPL_Ioctl()，从我第一次看API的时候，我就知道这绝对是一个反模式的 孽障，因为它几乎什么都能干，太恐怖了。

所以，我一直踌躇莫展，嗯，其实说起来，如果我早几天想到这个事情，也许我就不会挨K了~~~
现在想起来，从技术或者非技术的角度来看，出来混，迟早都是要还的，真是至理名言。

今天呢，其实，我的主要任务是写文档，否则又地挨K了。
但是鉴于昨晚我想着想着，突然想到了其实这正是个好机会实践重构，并且想到了一些看起来比较靠谱比较具有可操作性的方法，今天不做又实在于心不忍，于是我决定，边写文档边干这件事，然后就直播吧~~~

现在先写一下想到的那些比较靠谱的方法：
还是写在沙发贴吧，反正绝对没人跟我抢

辛昕

测试套件
   不管是马丁福勒（重构一书的作者），还是Kent Beck，以及测试驱动开发 的说法。一套测试套件是非常重要的——事实上，此前我一直一筹莫展的原因正是，我不知道该怎么为这个SMPL_Ping()函数写一套测试用例。
   原因是多方面的：
   尽管API里提供了这个函数的说明，然而，它并没有告诉我们应该怎么配套使用（很显然，对于无线的东西，特别是对于这个设计思路下的API，许许多多的API都是成对配套使用的。）
      而我粗粗观察过返回值，却发现它的返回值很奇怪。我束手无策。
   
    但是昨天晚上，我想到的事情恰好是反过来的，既然我不知道这个API具体要怎么成对配套使用，而这很可能就是我要深入实现代码内部理解的部分（马丁福勒使我相信，并且我发现我已经下意识养成了这个习惯，通过重构理解代码，特别是别人的代码。）
    但是，我仍然可以通过唯一可以进行的接口来进行测试。

    所以，第一步，我要增加的测试用例非常简单：
    1.针对不同的linkID,根据自己的理解和了解，我选择了三个等价类：
       1-1.默认，未建立的0；
       1-2.目前正常已经建立的ID号；
       1-3.0以外，但未曾正常建立的ID号;

     2.对于上面的1-2的一个细分情况： 那就是建立这个ID的另一个节点 存在 和 不存在的情况（断电或者上电）；

      我要观察这样调用时，返回值各是什么；
      同时，基于我并不保证每次调用都会返回相同的结果（这也是基于我先前做这部分调试时的一个经验）。
      所以返回结果我会存储在一个Trace里——我个人常干这种用缓冲存储一系列轨迹值得做法，因此我都取名为Trace;

辛昕

然后呢?
额，其实对于 重构这本书，我还特么不小心误解了 评注版 的意思，买了一本 英文版，我其实没看多少。

因此我并不知道具体的哪些方法是如何的。
但是我会凭着自己的直觉和方式进行的......

因此，对于然后干什么这个问题......
那，那当然是我接下来做了才知道啊~~~

至于现在，我的天，10点了，我还是赶紧开始写文档吧~~~不然又是挨K的节奏啊

辛昕

我一直都希望可以建立自动测试。
然而我面对的情况总是没那么容易。
比如前面提到的测试套件，实际上，是不可能自动测试的。

因为我需要 关闭/重启 另一台链接着的 开发板 ——不过，我倒是突然想到，这个其实是可以实现的。
做一个平台，供电不就行了？
嗯，这个主意不错的说。 先押下

实际上，我做的这个测试套件，呵呵，可以说，没什么新意。
不过我把这一部分独立抽取出来了，最终我实际嵌入代码的只有一句函数调用，倒也有点意义。

1. static smplStatus_t Trace[50];
   
2. static U8 TracePointer = 0;
   
3. 
   
4. void PingResult_Rec(smplStatus_t comeout)
   
5. {
   
6.      Trace[TracePointer++] = comeout;
   
7.      if(TracePointer >= 50)
   
8.         TracePointer = 0;
   
9. }

复制代码

1. 
   
2. smplStatus_t PingReplace(linkID_t lid);
   
3. 
   
4. void smplPing_Test(linkID_t TestID)
   
5. {
   
6.      PingResult_Rec(PingReplace(TestID));
   
7. }

复制代码

这里解释一下，这个PingReplace正是我要测试的 SMPL_Ping 的一个副本。
因为我要对它进行重构。

辛昕

1. /******************************************************************************
   
2. * @fn          nwk_ping
   
3. *
   
4. * @brief       Called from the application level to ping a peer. A small
   
5. *              payload is sent that includes a tid to detect correct reply.
   
6. *              Caller does not supply payload.
   
7. *
   
8. * input parameters
   
9. * @param   lid     - Link ID representing peer to ping
   
10. *
    
11. * output parameters
    
12. *
    
13. * @return   SMPL_SUCCESS   valid reply received
    
14. *           SMPL_TIMEOUT   no valid reply received
    
15. *           SMPL_NO_CHANNEL  no channels returned on a scan
    
16. */
    
17. smplStatus_t nwk_ping(linkID_t lid)
    
18. {
    
19.   connInfo_t  *pCInfo   = nwk_getConnInfo(lid);
    
20.   smplStatus_t rc       = SMPL_BAD_PARAM;
    
21.   uint8_t      done     = 0;
    
22.   uint8_t      repeatIt = 2;
    
23.   uint8_t      msg[MAX_PING_APP_FRAME];
    
24.   uint8_t      radioState = MRFI_GetRadioState();
    
25.   union
    
26.   {
    
27.     ioctlRawSend_t    send;
    
28.     ioctlRawReceive_t recv;
    
29.   } ioctl_info;
    
30. 
    
31.   if (!pCInfo || (SMPL_LINKID_USER_UUD == lid))
    
32.   {
    
33.     /* either link ID bogus or tried to ping the unconnected user datagram link ID. */
    
34.     return rc;
    
35.   }
    
36. 
    
37.   do
    
38.   {
    
39. #if defined(FREQUENCY_AGILITY) && !defined(ACCESS_POINT)
    
40.     uint8_t     i, numChan;
    
41.     freqEntry_t channels[NWK_FREQ_TBL_SIZE];
    
42. 
    
43.     if (repeatIt == 2)
    
44.     {
    
45.       /* If FA enabled, first time through set up so that the 'for'
    
46.        * loop checks the current channel. This saves time (no scan)
    
47.        * and is very likely to succeed. Populate the proper strucure.
    
48.        */
    
49.       SMPL_Ioctl(IOCTL_OBJ_FREQ, IOCTL_ACT_GET, channels);
    
50.       numChan = 1;
    
51.     }
    
52.     else
    
53.     {
    
54.       /* If we get here we must scan for the channel we're now on */
    
55.       if (!(numChan=nwk_scanForChannels(channels)))
    
56.       {
    
57.         return SMPL_NO_CHANNEL;
    
58.       }
    
59.     }
    
60.     /* Either we scan next time through or we're done */
    
61.     repeatIt--;
    
62. 
    
63.     /* this loop Pings on each channel (probably only 1) looking
    
64.      * for peer.
    
65.      */
    
66.     for (i=0; i
67.     {
    
68.       nwk_setChannel(&channels[i]);
    
69. #else
    
70.     {
    
71.       repeatIt = 0;
    
72. #endif  /* defined(FREQUENCY_AGILITY) && !defined(ACCESS_POINT) */
    
73. 
    
74.       ioctl_info.send.addr = (addr_t *)pCInfo->peerAddr;
    
75.       ioctl_info.send.msg  = msg;
    
76.       ioctl_info.send.len  = sizeof(msg);
    
77.       ioctl_info.send.port = SMPL_PORT_PING;
    
78. 
    
79.       /* fill in msg */
    
80.       msg[PB_REQ_OS] = PING_REQ_PING;
    
81.       msg[PB_TID_OS] = sTid;
    
82. 
    
83.       SMPL_Ioctl(IOCTL_OBJ_RAW_IO, IOCTL_ACT_WRITE, &ioctl_info.send);
    
84. 
    
85.       ioctl_info.recv.port = SMPL_PORT_PING;
    
86.       ioctl_info.recv.msg  = msg;
    
87.       ioctl_info.recv.addr = 0;
    
88. 
    
89.       NWK_CHECK_FOR_SETRX(radioState);
    
90.       NWK_REPLY_DELAY();
    
91.       NWK_CHECK_FOR_RESTORE_STATE(radioState);
    
92. 
    
93.       if (SMPL_SUCCESS == SMPL_Ioctl(IOCTL_OBJ_RAW_IO, IOCTL_ACT_READ, &ioctl_info.recv))
    
94.       {
    
95.         repeatIt = 0;
    
96.         done     = 1;
    
97.         sTid++;   /* guard against duplicates */
    
98.       }
    
99.     }
    
100.   } while (repeatIt);
     
101. 
     
102.   return done ? SMPL_SUCCESS : SMPL_TIMEOUT;
     
103. 
     
104. }

复制代码

辛昕

这个函数看起来很长很复杂。
所以，即使不看重构的那些具体方法，我也一定会选择分而治之。
分解成几个小模块。

一直以来，我都非常不爽这种用宏来开关一部分源码的方式
——后来我考虑过，即便我要这样做，那我也会选择把这一部分做成一个独立的函数，然后在这个Ping函数里调用这个函数，而非直接把实现写在这里。

再考虑这个 宏。
（你们不知道具体的细节，所以没办法了解，且听我说就好）
#if defined(FREQUENCY_AGILITY) && !defined(ACCESS_POINT)
FA是指频率调整，然而我这个项目里并不需要，至少暂时不需要，为了简单，现在我决定忽略它。

于是这一部分，在PingReplace里直接去掉。
去掉以后，你会发现一个小细节——这种事，坦白说我干过，所以我知道它有多么不好，代码既不优雅，还容易出错——我就出过 ，错在{}这个括号个数不匹配。

与此同时，我们再看代码，就发现。
这样之下，repeatIt已经没意义了。
于是去掉。
同时 包在最外头的 do-while结构也没用了。

这个时候，我们就发现了，实际上，下面最重要的工作是一组基于SMPL_IOCTL的 收发确认。
为了简便，更集中在这个点上——
实际上此前，我利用上面的那个 测试套件，没有怎么分析的情况下，我发现问题的关键以及功能 都是靠这部分实现。

因此上面写的其实是一个结果。

最后得到这样一个函数。

1.   typedef union
   
2.   {
   
3.     ioctlRawSend_t    send;
   
4.     ioctlRawReceive_t recv;
   
5.   }Ioctl_InfoStr;
   
6. 
   
7. uint8_t Ping_SendAndRecv(connInfo_t *pCInfo)
   
8. {
   
9.       Ioctl_InfoStr ioctl_info;
   
10.       uint8_t      msg[MAX_PING_APP_FRAME];
    
11.       uint8_t      radioState = MRFI_GetRadioState();
    
12.    
    
13.       ioctl_info.send.addr = (addr_t *)pCInfo->peerAddr;
    
14.       ioctl_info.send.msg  = msg;
    
15.       ioctl_info.send.len  = sizeof(msg);
    
16.       ioctl_info.send.port = SMPL_PORT_PING;
    
17. 
    
18. 
    
19.       msg[PB_REQ_OS] = PING_REQ_PING;
    
20.       msg[PB_TID_OS] = sTid;
    
21. 
    
22.       SMPL_Ioctl(IOCTL_OBJ_RAW_IO, IOCTL_ACT_WRITE, &ioctl_info.send);
    
23. 
    
24.       ioctl_info.recv.port = SMPL_PORT_PING;
    
25.       ioctl_info.recv.msg  = msg;
    
26.       ioctl_info.recv.addr = 0;
    
27. 
    
28.       NWK_CHECK_FOR_SETRX(radioState);
    
29.       NWK_REPLY_DELAY();
    
30.       NWK_CHECK_FOR_RESTORE_STATE(radioState);
    
31. 
    
32.       if (SMPL_SUCCESS == SMPL_Ioctl(IOCTL_OBJ_RAW_IO, IOCTL_ACT_READ, &ioctl_info.recv))
    
33.         return 1;
    
34.       else
    
35.         return 0;
    
36. }
    
37. 
    
38. 
    
39. 
    
40. /******************************************************************************
    
41. * @fn          nwk_ping
    
42. *
    
43. * @brief       Called from the application level to ping a peer. A small
    
44. *              payload is sent that includes a tid to detect correct reply.
    
45. *              Caller does not supply payload.
    
46. *
    
47. * input parameters
    
48. * @param   lid     - Link ID representing peer to ping
    
49. *
    
50. * output parameters
    
51. *
    
52. * @return   SMPL_SUCCESS   valid reply received
    
53. *           SMPL_TIMEOUT   no valid reply received
    
54. *           SMPL_NO_CHANNEL  no channels returned on a scan
    
55. */
    
56. smplStatus_t PingReplace(linkID_t lid)
    
57. {
    
58.   connInfo_t  *pCInfo   = nwk_getConnInfo(lid);
    
59.   smplStatus_t rc       = SMPL_BAD_PARAM;
    
60.   uint8_t      done     = 0;
    
61.   
    
62.   if (!pCInfo || (SMPL_LINKID_USER_UUD == lid))
    
63.   {
    
64.     /* either link ID bogus or tried to ping the unconnected user datagram link ID. */
    
65.     return rc;//
    
66.   }
    
67.   
    
68.   done = Ping_SendAndRecv(pCInfo);
    
69.   if(done == 1)
    
70.      sTid++;
    
71. 
    
72.   return done ? SMPL_SUCCESS : SMPL_TIMEOUT;
    
73. }
    

复制代码

辛昕

然后......
然后正当我刚开始挖出要重点弄得点的时候......
嗯，俺要下马了。

之前boss问方向，俺选择了win32位。
于是现在就要弄CBuilder

之前还想着一边弄这个东东。
不过两星期要交出一个串口助手，看来有点痛苦。
回想起这几天赶地痛苦，还是要继续靠谱才行啊~~

至于这个东东，额~~
真不知道会什么时候开始了~~~

CC2530，即将从正职变成业余。

也就是说，业余
在uS,在PSoC4(当然这个只会持续一段时间).....
以外，又多了一个 CC2530？！

不过，额，其实说起来，本来我就是想弄弄上位机，进而到数据库什么的......

突然有一种略微奇怪的感觉有木有~~