Partial fix for Jira issue IOT-24.

    Upon a successful observe registration/dereg new return values have been added.
    These values are for success cases and need to be considered along with OC_STACK_OK.

    Patch set #2:
    1) ObserveReq struct has a member holding the result of registering and observer
    2) OCStackResult has only one success code possible

    Change-Id: I3b6fc51d64e602b8dbbca85e0e669fd7d7d9f230

Patch Set #3:
1) responded to comments
2) rebased on master

Change-Id: I2ee4d6a641b9b17adf69efca5da75cf3a4e4d3b7

diff --git a/csdk/occoap/src/occoap.c b/csdk/occoap/src/occoap.c
index 1159f78..38b34c8 100644 (file)
--- a/csdk/occoap/src/occoap.c
+++ b/csdk/occoap/src/occoap.c
@@ -94,7 +94,7 @@ static void HandleCoAPAckRst(struct coap_context_t * ctx, uint8_t msgType,
     if(msgType == COAP_MESSAGE_RST){
         // now the observer should be deleted
         result = OCObserverStatus(sentToken, OC_OBSERVER_NOT_INTERESTED);
-        if(result == OC_STACK_OBSERVER_REMOVED){
+        if(result == OC_STACK_OK){
             OC_LOG_V(DEBUG, TAG, "Received RST, removing all queues associated with Token %d bytes",sentToken->tokenLength);
             OC_LOG_BUFFER(INFO, TAG, sentToken->token, sentToken->tokenLength);
             coap_cancel_all_messages(ctx, &sentQueue->remote, sentToken->token,
@@ -181,21 +181,27 @@ static void HandleCoAPRequests(struct coap_context_t *ctx,
              whether it is a sequence number or registration option!------------");
     OC_LOG_V(INFO, TAG, "Response from ocstack: %s", request->entityHandlerRequest->resJSONPayload);
 
-    switch(responseResult)
+    if(rcvdObsReq)
+    {
+        switch(rcvdObsReq->result)
+        {
+        case OC_STACK_OK:
+            observeOption = rcvdObsReq->option;
+            result = FormResponseOptList(&optList, &mediaType, &maxAge, 0, NULL);
+            break;
+        case OC_STACK_OBSERVER_NOT_ADDED:
+        case OC_STACK_OBSERVER_NOT_REMOVED:
+        case OC_STACK_INVALID_OBSERVE_PARAM:
+        default:
+            result = FormResponseOptList(&optList, &mediaType, &maxAge, 0, NULL);
+            break;
+        }
+    }
+    else
     {
-    case OC_STACK_OBSERVER_ADDED:
-        observeOption = OC_RESOURCE_OBSERVE_REGISTER;
-        result = FormResponseOptList(&optList, &mediaType, &maxAge, 0, NULL);
-        break;
-    case OC_STACK_OBSERVER_REMOVED:
-        observeOption = OC_RESOURCE_OBSERVE_DEREGISTER;
-        result = FormResponseOptList(&optList, &mediaType, &maxAge, 0, NULL);
-        break;
-    case OC_STACK_OK:
-    default:
         result = FormResponseOptList(&optList, &mediaType, &maxAge, 0, NULL);
-        break;
     }
+
     VERIFY_SUCCESS(result, OC_STACK_OK);
 
     // generate the pdu, if the request was CON, then the response is ACK, otherwire NON

diff --git a/csdk/occoap/src/occoaphelper.c b/csdk/occoap/src/occoaphelper.c
index ea00494..54552ab 100644 (file)
--- a/csdk/occoap/src/occoaphelper.c
+++ b/csdk/occoap/src/occoaphelper.c
@@ -43,8 +43,6 @@ uint8_t OCToCoAPResponseCode(OCStackResult result)
     uint8_t ret;
     switch(result)
     {
-        case OC_STACK_OBSERVER_ADDED :
-        case OC_STACK_OBSERVER_REMOVED :
         case OC_STACK_OK :
             ret = COAP_RESPONSE_200;
             break;
@@ -260,6 +258,7 @@ OCStackResult FormOCObserveReq(OCObserveReq ** observeReqLoc, uint8_t observeOpt
     observeReq->option = observeOption;
     observeReq->subAddr = remote;
     observeReq->token = rcvdToken;
+    observeReq->result = OC_STACK_OK;
 
     *observeReqLoc = observeReq;
     return OC_STACK_OK;
@@ -588,7 +587,7 @@ observation:
     }
 
     result = OCObserverStatus(token, OC_OBSERVER_FAILED_COMM);
-    if(result == OC_STACK_OBSERVER_REMOVED)
+    if(result == OC_STACK_OK)
     {
         coap_cancel_all_messages(ctx, &queue->remote, token->token, token->tokenLength);
     }

diff --git a/csdk/stack/include/internal/ocstackinternal.h b/csdk/stack/include/internal/ocstackinternal.h
index 37557e0..5d67dd4 100644 (file)
--- a/csdk/stack/include/internal/ocstackinternal.h
+++ b/csdk/stack/include/internal/ocstackinternal.h
@@ -131,6 +131,8 @@ typedef struct {
     OCDevAddr *subAddr;
     // CoAP token for the observe request
     OCCoAPToken *token;
+    // The result of the observe request
+    OCStackResult result;
 } OCObserveReq;
 
 // following structure will be created in occoap and passed up the stack on the server side

diff --git a/csdk/stack/include/ocstack.h b/csdk/stack/include/ocstack.h
index f00a1cd..49cf0d8 100644 (file)
--- a/csdk/stack/include/ocstack.h
+++ b/csdk/stack/include/ocstack.h
@@ -113,8 +113,8 @@ typedef enum {
     OC_STACK_SLOW_RESOURCE,
     OC_STACK_NO_OBSERVERS, /* resource has no registered observers */
     OC_STACK_OBSERVER_NOT_FOUND,
-    OC_STACK_OBSERVER_ADDED,
-    OC_STACK_OBSERVER_REMOVED,
+    OC_STACK_OBSERVER_NOT_ADDED,
+    OC_STACK_OBSERVER_NOT_REMOVED,
     OC_STACK_ERROR
 } OCStackResult;
 

diff --git a/csdk/stack/src/ocobserve.c b/csdk/stack/src/ocobserve.c
index a298754..93687fa 100644 (file)
--- a/csdk/stack/src/ocobserve.c
+++ b/csdk/stack/src/ocobserve.c
@@ -48,10 +48,13 @@ OCStackResult OCObserverStatus(OCCoAPToken * token, uint8_t status)
         OC_LOG(DEBUG, TAG, PCF("observer is not interested in our notifications anymore"));
         //observer is dead, or it is not observing anymore
         result = DeleteObserver (token);
-        if(result == OC_STACK_OK)
+        if(result != OC_STACK_OK)
+        {
+            result = OC_STACK_OBSERVER_NOT_REMOVED;
+        }
+        else
         {
             OC_LOG(DEBUG, TAG, PCF("removing an observer"));
-            result = OC_STACK_OBSERVER_REMOVED;
         }
         break;
     case OC_OBSERVER_STILL_INTERESTED:
@@ -79,13 +82,19 @@ OCStackResult OCObserverStatus(OCCoAPToken * token, uint8_t status)
             if(observer->failedCommCount >= MAX_OBSERVER_FAILED_COMM)
             {
                 result = DeleteObserver (token);
-                OC_LOG(DEBUG, TAG, PCF("removing an observer"));
-                result = OC_STACK_OBSERVER_REMOVED;
+                if(result != OC_STACK_OK)
+                {
+                    result = OC_STACK_OBSERVER_NOT_REMOVED;
+                }
+                else
+                {
+                    OC_LOG(DEBUG, TAG, PCF("removing an observer"));
+                }
             }
             else
             {
                 observer->failedCommCount++;
-                result = OC_STACK_OK;
+                result = OC_STACK_OBSERVER_NOT_REMOVED;
             }
             observer->forceCON = 1;
             OC_LOG_V(DEBUG, TAG, "Failed count for this observer is %d",observer->failedCommCount);
@@ -100,7 +109,7 @@ OCStackResult OCObserverStatus(OCCoAPToken * token, uint8_t status)
 
 OCStackResult ProcessObserveRequest (OCResource *resource, OCRequest *request)
 {
-    OCStackResult stackRet = OC_STACK_ERROR;
+    OCStackResult stackRet = OC_STACK_OK;
     OCEntityHandlerResult ehRet = OC_EH_ERROR;
     OCEntityHandlerRequest *ehReq = request->entityHandlerRequest;
     OCObserveReq *obs = request->observe;
@@ -118,28 +127,35 @@ OCStackResult ProcessObserveRequest (OCResource *resource, OCRequest *request)
             // TODO: we need to check if the obsrever is already there using its OCDevAdd....
             stackRet = AddObserver ((const char*)(request->resourceUrl), (const char *)(ehReq->query),
                     obs->token, obs->subAddr, resource, request->qos);
-            if(stackRet == OC_STACK_OK)
+            if(stackRet != OC_STACK_OK)
+            {
+                obs->result = OC_STACK_OBSERVER_NOT_ADDED;
+            }
+            else
             {
-                stackRet = OC_STACK_OBSERVER_ADDED;
+                OC_LOG(DEBUG, TAG, PCF("adding an observer"));
             }
-            OC_LOG(DEBUG, TAG, PCF("adding an observer"));
         }
         else if (obs->option == OC_RESOURCE_OBSERVE_DEREGISTER)
         {
             // Deregister observation
             stackRet = DeleteObserver (obs->token);
-            if(stackRet == OC_STACK_OK)
+            if(stackRet != OC_STACK_OK)
+            {
+                obs->result = OC_STACK_OBSERVER_NOT_REMOVED;
+            }
+            else
             {
                 OC_LOG(DEBUG, TAG, PCF("removing an observer"));
-                stackRet = OC_STACK_OBSERVER_REMOVED;
             }
         }
         else
         {
             // Invalid option
             OC_LOG(ERROR, TAG, PCF("Invalid CoAP observe option"));
-            stackRet = OC_STACK_INVALID_OBSERVE_PARAM;
+            obs->result = OC_STACK_INVALID_OBSERVE_PARAM;
         }
+        stackRet = OC_STACK_OK;
     }
     else
     {
@@ -180,7 +196,6 @@ OCStackResult SendObserverNotification (OCResource *resPtr)
 
             // Even if entity handler for a resource is not successful
             // we continue calling entity handler for other resources
-            //ehRet = resPtr->entityHandler (OC_REQUEST_FLAG, entityHandlerReq);
             ehRet = BuildObsJSONResponse((OCResource *) resPtr, entityHandlerReq);
             if (OC_EH_OK == ehRet)
             {

diff --git a/csdk/stack/src/ocresource.c b/csdk/stack/src/ocresource.c
index a7e99c7..a498798 100644 (file)
--- a/csdk/stack/src/ocresource.c
+++ b/csdk/stack/src/ocresource.c
@@ -461,22 +461,6 @@ BuildJSONResponse(ResourceHandling resHandling, OCResource *resource, OCRequest
                 break;
             }
 
-        case OC_RESOURCE_NOT_SPECIFIED:
-            // This case is not needed as the logic changed so OCCancel results in RESET
-            // rather than a GET. RESET is handled at lower layers.
-
-            // TODO: This is a special case. In M1 this occurs only for observation
-            // delete since OCCancel (on the client) only takes OCDoHandle param.
-            // TODO: Remove comments below before release - only for code review
-            // OPEN: We had decided to revisit the OCDoHandle logic for this sprint. If it
-            // changes and URI is passed this special case will not be needed.
-            OC_LOG(INFO, TAG, PCF("OC_RESOURCE_NOT_SPECIFIED"));
-            if (request->observe != NULL)
-            {
-                ret = ProcessObserveRequest (resource, request);
-            }
-            break;
-
         default:
             {
                 OC_LOG(INFO, TAG, PCF("Invalid Resource Determination"));