#32 Updates to allow OpenOBEX to talk to a C-Pen 800C

v1.7.2 or v1.8
open-accepted
None
5
2014-02-21
2014-02-09
tgaz
No

Arguably, the Continue patch may break for users relying on undocumented/unspecified behavior. The other two patches should be straight-forward improvements.

3 Attachments

Discussion

  • I applied the patches #1 and #3 but I cannot quite agree with patch #2.

    1. The check is wrong as as multi-packet reponse at this code location can only be a GET request, never a PUT request.
    2. A similar check for server side is missing.

    Additionally, what are those "Some user-defined commands abuse Continue".

     
  • tgaz
    tgaz
    2014-02-10

    1. OK, will have a look.
    2. I didn't think of the server side, as I didn't need it. Will have a look.

    A pen scanner I have uses proprietary protocol over OBEX over IrLAP over RS-232/IrDA, but doesn't support any OBEX commands except CONNECT and DISCONNECT. All other requests use the user-defined range. Unfortunately they seem to return Continue rather than Success for every request. It essentially means I have to let the application layer handle the continue logic as it is on a case-to-case basis if it makes sense or not.

    It is a bit hacky, I know, but I think overall it makes sense for OpenOBEX not to make make assumptions about behavior that is unspecified.

     
    • assigned_to: Hendrik Sattler
    • Group: -->
     
  • Nice :-)

    Actually, IrDA is a kind of transport that is hard to support these days. I have no current hardware that has IrDA. But I will add all the fixes needed to make this hardware work for you :-)

    Regards...

     
  • tgaz
    tgaz
    2014-02-10

    I've added the same code for the server. (I have no idea what it means for a server to receive a response, so I'll just assume this was the right spot.)

    Looking at the code, I don't see why only GET could reach that piece. I'm (trying to) follow IrDA OBEX 1.2, and it seems pretty symmetric when it comes to Continue handling in GET and PUT. If you have any other info, I'm all-ears. :)

    I found another issue today with IrOBEX_TransportConnect() not always returning error when it should. (It caused busy-wait in HandleInput() later.) Attached that new patch.

     
  • Well, *_request_rx|tx are for the request sending state. From the client's POV, only PUT requests can take more than one OBEX part. IIRC, all other commands from OBEX base specification are required to contain all request parameters in the first packet.

    The *_response_rx|tx are for the chatting after the request finished sending. From the client's POV, only GET requests can have more than one part here.

    If I'm thinking this wrong, then all types are allowed for CONTINUE and your patch is wrong ;-)

     
  • tgaz
    tgaz
    2014-02-13

    Apologies for a long post, but here is my reasoning. Short version: We probably agree on GET and PUT, but not how it generalizes to other request types.

    I think we agree that *_response_rx() is called for all responses sent by the remote end, no matter what command was used. At least, this is my understanding.

    We then have three cases: GET, PUT, and others. I'm only talking of *_response_rx() here since that is where Continue is being handled.

    For GET, we agree that any non-last response can be Continue, and then we move back to sending a "continuation request," which is basically going through all non-sent headers and sends them in a new request. For GET, there are no new headers (by specification), so the continuation request is empty. This goes on until a request with the final bit is sent. Then the server responds with OK.

    For PUT, I argue we have the same case, just that usually the queue of unsent headers is not empty. The server will send Continue, and the client will respond by sending a continuation request, just like it does for GET. This goes on until the client sends a final PUT, in which case the server responds with OK. This is from the PUT example on p. 28 of IrOBEX 1.2. I think we agree even on this case, but we are not on the same page on how that generalizes to other requests.

    So, for GET and PUT (but no others) there are specific details on what should happen when the server sends Continue (sections 3.3.3.2 and 3.3.4). The IrOBEX 1.2 specification mandates a continuation request if a Continue is seen for GET and PUT. However, it does not state that Continue should be handled that way for any other request types. That is left up to the implementation, it seems.

    In my case, I have a device that sends Continue even though the host should not immediately send a continuation request. This particular request is a poll to see if there is more data, so I want to send it, receive a reply and then wait some fraction of a second before I send the next request. In a way, they are continuation requests with delays.

    This could be implemented in OpenOBEX as a user-configurable policy on when to schedule continuation requests (essentially a rate-limit,) but since the specification does not say how to handle Continue in general, I think it makes more sense for OpenOBEX to simply do hands-off for any request not GET or PUT.

     
  • Ah, now I see what you want to achieve. Stopping is already managable using suspend/resume. However, a callback event at the right moment (before obex_client_response_tx_prepare() is called) is missing, so that you can call OBEX_SuspendRequest()

     
  • tgaz
    tgaz
    2014-02-13

    :)

    Looking at the code, I agree suspending right before handling Continue would probably solve my issue. And to clean up the request, CancelRequest() seems valid at that point. (Since the scanner never sends OK, I need a way to just stop when I need to send other requests.)

    If you prefer that solution, I can change the patch to insert a new event delivery instead. Any name preference for the event? I see server sends PROGRESS , but that's after the Continue decision. So maybe just something like RESPRECVD right before the if-statements?

     
  • How about an OBEX_EV_CONTINUE right after the "if (rsp == OBEX_RSP_CONTINUE) {" line? Some checks for suspend and abort are probably needed at some places :-)

    I applied patch #4 to my (currently still local) git repository.

     
  • tgaz
    tgaz
    2014-02-14

    Here is a mostly untested version. It works with CancelRequest(nice=false).

    I have yet to figure out good ways to test SuspendRequest() and CancelRequest(true).

    The general idea is that

    • object==NULL could happen, and that's bad.
    • For object->abort==true, response_tx_prepare() handles it for us.
    • For object->suspended==true, object_append_data() handles it for us, but it will probably cause response_rx() to return RESULT_ERROR, which isn't very nice. OTOH that seems to follow the general pattern for object->suspended==true.
     
  • tgaz
    tgaz
    2014-02-18

    I have no way of testing SuspendRequest() as the device doesn't support GET/PUT-like continuations. It seems a bit hairy to get right with the state/direction to avoid infinite work loops, etc., so I'm simply stating only CancelRequest() is allowed.

     
    Last edit: tgaz 2014-02-18
  • tgaz
    tgaz
    2014-02-18

    Final attempt. Really scoped down to my needs. Adds EV_CONTINUE (client-side) and allows you to do CancelRequest(). I couldn't test the nice==true case fully since no request handles ABORT (it simply doesn't respond.) Sending it works, and I think that's good enough testing. Those 0.0001% out there who need this functionality can hopefully improve the code if needed. ;)

     
  • The 4 patches of yours are now in the git repository at gitorious.org/openobex.

    Thanks

     
    • status: open --> closed-accepted
     
    • Group: --> v1.7.2 or v1.8
     
    • status: closed-accepted --> open-accepted