ccollins476ad commented on pull request #2317:
URL: https://github.com/apache/mynewt-core/pull/2317#issuecomment-644905537


   Thanks @vrahane and @kasjer.
   
   > Code is fine, could you explain however why this is needed. If this is 
obvious for oic then forget I asked. I was just curious why code that work OK 
in one way now has to be changed.
   
   I briefly rationalized this change in my commit message, but I probably 
could have gone into more detail.  In the current code (without this PR), 
interfaces can fail to initialize and the application gets no feedback about 
the failure.  For example, say a device acts as a CoAP server with two 
interfaces: 1) serial, and 2) Bluetooth.  The call to `oc_connectivity_init()` 
can have four possible outcomes:
   
   | Serial  | Bluetooth | Return code |
   | ------- | --------- | ----------- |
   | Success | Success   | Success     |
   | Success | Fail      | Success     |
   | Fail    | Success   | Success     |
   | Fail    | Fail      | Fail        |
   
   To me this just seems wrong.  If an application expects two interfaces to be 
available, it should be notified if one of them fails.  Perhaps the ideal 
solution would be to implement a more complicated feedback mechanism such that 
the application knows exactly which interfaces succeeded and which ones failed, 
but that seemed like overkill to me.  Absent a more sophisticated feedback 
mechanism, I think it is best to err on the side of failure.  So my PR changes 
`oc_connectivity_init()` to use the below table instead:
   
   | Serial  | Bluetooth | Return code |
   | ------- | --------- | ----------- |
   | Success | Success   | Success     |
   | Success | Fail      | Fail        |
   | Fail    | Success   | Fail        |
   | Fail    | Fail      | Fail        |
   
   As for why the current code now needs to be changed: I think no one noticed 
this until now!  I think I may be the first person to have an interface fail to 
initialize.  The reason I made this change was to prevent others from dealing 
with the frustrating debugging experience that I just had :).  One interface 
failed to initialize for a weird reason (a mempool buffer wasn't properly 
aligned which caused os_mempool_init to fail, causing an interface's init 
function to fail) but there was no indication of the failure.  Much later, CoAP 
commands sent through the failed interface weren't getting processed.
   
   All that said, I am definitely OK with keeping things as-is if you feel this 
change is not an improvement :).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to