use existing check_wait_set_for_data to avoid duplicated code

[jwang11]

There is already function check_wait_set_for_data over there which can
help judge hasToWait, no need duplicated check one by one.
The logic is hasToWait = false if hasData is true, otherwise
keep hasToWait as it was.

Signed-off-by: jwang jing.j.wang@intel.com

[clalancette]

This looks reasonable to me. I've kicked off CI here:

• Linux
• Linux-aarch64
• macOS
• Windows

[clalancette]

@jwang11 FYI, the linters don't like the blank space on the end of the newline. I fixed it up in 2a4b09e. New CI:

• Linux
• Linux-aarch64
• macOS
• Windows

[clalancette]

The Linux builds look good, and the Windows build warning is a known failure. The OSX failure seems new, though, and I can't correlate it to a known flaky test with a quick look at the nightlies. So I'm not sure what is going on there.

[dhood]

I'd like to have a look at this before it gets merged because I think there's a reason they all need to be checked (while check_wait_set_for_data can return early)

[jwang11]

From my point, it is not efficient to check them all if there is one who already set hasToWait=true. Thus, use check_wait_set_for_data can slightly improve latency time in some cases.

[dhood]

There's an opportunity for code reuse here but the code that has been replaced is not exactly the same, so this changes behaviour:

https://github.com/ros2/rmw_fastrtps/pull/185/files#diff-8044b1eed73db087f43f89fe2967b7ffL186 was not clearing the guard condition but now check_wait_set_for_data will clear a guard condition. This means that when we check hasTriggered later on, the guard condition that got cleared will say that it hadn't triggered.

---

It should be fine to update check_wait_set_for_data to use hasTriggered instead, which will make it a direct replacement. The flag will still be cleared when we detach the guard condition, which is probably a better place for it to get cleared anyway. That shouldn't have any side effects since the hasData check has been moved up in #158. The only other change that will then be required is to remove this comment https://github.com/ros2/rmw_fastrtps/pull/158/files#diff-8044b1eed73db087f43f89fe2967b7ffR199

[jwang11]

@dhood so you mean I should update check_wait_set_for_data function using hasTriggered instead of hasData?

[dhood]

@jwang11 what I meant in my last comment was to replace getHasTriggered with hasTriggered. Looking at it again, however, I think I got the two mixed up: hasTriggered is currently already used within check_wait_set_for_data, so this code actually is functionally equivalent as is.

I kicked off another CI since the tests that failed last time are flaky.

• Linux
• Linux-aarch64
• macOS
• Windows

so this LGTM