Wait functions

[mikepurvis]

While the timespec PR is waiting on reviews, I want to get the ball rolling on this one, which is the actual change that one sets up for, based on the discussion in #37. Click here for a diff showing just the additional changes in this PR.

Haven't had a chance to test much at this point, but it will be going into battle on two different peripheral drivers in the next day or two, so there will be some practical miles on it long before an actual merge happens.

For the moment, it's also missing the stubs in the Windows implementation; compilation there will fail until those are added in.

---

Okay, the timespec PR is merged, and this one has been rebased accordingly.

[mikepurvis]

Key to note is that waitReadable does not differentiate between exiting due to interruption and exiting due to timeout—both cases return false. This happens to work out okay for the use in read(), since the timeout is detected immediately in the next trip around the loop, but it's something which deserves a bit of thought and consideration.

Another option might be waitAvailable instead of waitReadable, and have the function return a ssize_t which is the number of characters available to read, or zero for timeout, or -1 for a select error.

A further way to go would be having waitReadable internally maintain its own MillisecondTimer, and loop over the EINTR case, so that the only two possible exits are "readable", "timeout", and "IO exception thrown". This may be an unnecessary increase in complexity and duplication, but is another route to potentially consider.

[mikepurvis]

Since these changes break ABI, I'm assuming we're looking at Indigo for this stuff?

[wjwwood]

Yeah, I hadn't decided about how to roll out these changes, but you are correct, that it should probably be a 1.2 release.

[wjwwood]

Does this one now need a rebase?

[mikepurvis]

Yes, rebased now.

[mikepurvis]

Any further thought given to merging this? We've been successfully using it for several months in the Roboteq and UM6 drivers and it seems to be working as expected.

I believe the only missing piece is stubs for the Windows implementation, which I can provide.

[wjwwood]

I haven't had time to, we will have to have the Windows functions, and I haven't decided how to role this out. My instinct is to put it into 1.2 with other ABI breaking changes and release it into Indigo. Since Indigo farms are still being worked out that may be some time yet, so there hasn't been any urgency yet.

Also, have you given any more thought to this point?

Key to note is that waitReadable does not differentiate between exiting due to interruption and exiting due to timeout—both cases return false. This happens to work out okay for the use in read(), since the timeout is detected immediately in the next trip around the loop, but it's something which deserves a bit of thought and consideration.

[mikepurvis]

I think the waitReadable implementation as it stands is okay. The main envisioned use is when you want a thread that does nothing but block on the receipt of serial messages—but also jumps out of the blocking state periodically to perform other maintenance.

In that scenario, the only thing that matters is the true/false of "is there data ready to read"—whether the function is capable of returning in advance of the specified timeout is a matter for documentation. If the timing of the code before and after this point is critical, then the user will have implement their own external scheduling, not depend on that within waitReadable.

[wjwwood]

I'm sorry I have just had zero time to look at this lately. Really overwhelmed right now.

Anyways, I would consider this good to go for 1.2.x if the Windows stubs (or better yet implementations) were there.

[mikepurvis]

Okay, stubs added! I don't have a way of testing or even compiling them, but they should be about right.

[wjwwood]

I can test them, thanks!

[mikepurvis]

Any chance we'll get Serial 1.2 with Indigo?

[wjwwood]

I could probably do that, I have some outstanding changes, like this one that need attention. I'll do my best.

[mikepurvis]

Okay, thanks! Still would love to help out with a better test harness for it at some point, but we're all pretty busy, obviously. :)

[wjwwood]

On windows, I think you are missing this:

diff --git a/include/serial/impl/win.h b/include/serial/impl/win.h
index 605f5d0..87d7bb8 100644
--- a/include/serial/impl/win.h
+++ b/include/serial/impl/win.h
@@ -75,6 +75,12 @@ public:
   size_t
   available ();

+  bool
+         waitReadable(uint32_t timeout);
+
+  void
+         waitByteTimes(size_t count);
+
   size_t
   read (uint8_t *buf, size_t size = 1);

Sorry for just giving you a diff, but I am sort of handicap on Windows.

[wjwwood]

I think that if you can update the pull request with the above diff, then I can merge this for inclusion in 1.2 and Indigo.

[mikepurvis]

Okay, I think that should do.