Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add request/response API to ORSSerialPort #17

Closed
alexandre-normand opened this issue Jan 31, 2014 · 7 comments
Closed

Add request/response API to ORSSerialPort #17

alexandre-normand opened this issue Jan 31, 2014 · 7 comments
Assignees
Milestone

Comments

@alexandre-normand
Copy link

Let me preface this issue by saying that I might be wrong. I noticed this last night and I haven't had the chance to debug this properly (I can do more digging tonight).

I'm interacting with a device that might have responses that are relatively large (maybe 5k bytes?). Ideally, my delegate would receive the NSData for the full response but I'm getting only a chunk of it. The read buffer is currently 1024 bytes long but we could potentially read until there are no more bytes left to read, right?

Or is the idea that the delegate would accumulate bytes and put the response back together?

@armadsen
Copy link
Owner

I'm on vacation and can't look into this in depth at the moment to verify 100% that it's working as intended. Off the top of my head, I believe it is. Currently the assumption is indeed that the delegate is responsible for accumulating bytes, determining when a full packet (whatever that means in the context of your system) has come in, then acting on it.

I have been thinking about some enhancements to ORSSerialPort so that port objects themselves could accumulate and buffer bytes, with e.g. a user/client supplied block to determine when a full packet has come in. This isn't a trivial idea from an API design standpoint, so I'm still thinking about it.

Anyway, I'll look into this more thoroughly when I'm back from vacation next week.

@ghost ghost assigned armadsen Jan 31, 2014
@alexandre-normand
Copy link
Author

Thanks Andrew. Feel free to close the issue if you think that shouldn't be supported. If you do want to think about adding support for such a feature though, maybe it could be made simpler by having an additional callback on the delegate that receives fully read packets from a port. The loop that reads from the port would then both call delegates on each chunk being read but it could read additional packets until no more bytes are available, then call a new receivedPacket callback and reset the state to start a new packet.

The idea being that this would work well for request/response interactions.

I'm being totally selfish here because it's my use-case. 😉

@armadsen
Copy link
Owner

I'm going to turn this into an issue to discuss adding request/response support to ORSSerialPort itself. It has come up a few times since this issue was opened, and seems like it would be useful to a lot of people. In brief, it should be something like this (subject to change):

  • ORSSerialPort will buffer incoming data so that a complete packet can be delivered to the delegate when it's ready.
  • Allow the sender of data to somehow specify what a "complete packet" means, giving ORSSerialPort a way to determine when buffering can stop and data can be delivered.
  • Allow for a timeout so that if a complete packet is never received, the port is not "hung".
  • (Possibly) give ORSSerialPort a way to maintain the association between a request and a response so the sender/delegate can know which request an incoming packet was a response to.
  • (Possibly) Optionally disallow sending data while a request is pending.

@alexandre-normand
Copy link
Author

I like this plan. Keeping track of requests is also very handy. I ended up implementing all of this (except for disallowing the sending of data) on my side but this would have made it way simpler for me if ORSSerialPort had had this feature.

@armadsen
Copy link
Owner

I've begun work on this on the RequestResponse branch. It's still very early on, but feel free to provide input if you have any.

@armadsen
Copy link
Owner

I didn't really mean to close this...

@armadsen
Copy link
Owner

armadsen commented Nov 3, 2014

This was implemented by #26.

@armadsen armadsen closed this as completed Nov 3, 2014
@armadsen armadsen added this to the 1.5.0 milestone Sep 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants