Fixed read timeout checks, added timeout parameter

[gsokoll]

The QBaseSerialWitmotionSensorReader::ReadData() function performs a check to detect if no data has been received for a period of time. Specifically, it checks whether the number of bytes currently available from the serial stream is the same as the number of bytes available on the previous call to the function, and if this occurs several times it raises an error.

However the logic of this check is incorrect - the number of bytes available now and in the past is not an indication of the absence of data. The code has been amended to check whether there are ZERO bytes available on the serial stream, and if this occurs over a specified timeout period, it raises an error.

The timeout period has been added as a parameter also.

[twdragon]

@gsokoll this PR seemed to copy a lot of commits. Please rebase the PR to start the review without including the previous commits which are not made by you

[gsokoll]

I had made the PR against "master" instead of "main" branch. Fixed now. Can you perhaps delete the old master branch also?

[twdragon]

@gsokoll I agreed with my colleagues to keep it for historical purposes. However, thanks for rebasing. I will need some time to review the changes.

[twdragon]

@gsokoll did you see the changes I listed here?

[gsokoll]

@gsokoll did you see the changes I listed here?

No, I don't see any changes. Where were they made??

[twdragon]

@gsokoll I meant the review I opened before. Please let's resolve it before merging

[gsokoll]

Sorry I'm not very familiar with github review process. Looks like only you can review the PR?

[twdragon]

@gsokoll no, everything is fine. We should just resolve the discussion on the line I started here, after that I will merge the PR. I am just kindly asking you if you consider keeping the existing behaviour of the node if the timeout is not specified.

[gsokoll]

Sorry I am still missing something. I do not see any comments in the "Files changed" tab.

[twdragon]

Please refer here, just above

[gsokoll]

Ah, ok. I think I can't see your comments because they are still Pending (https://github.com/orgs/community/discussions/10369) ?

But in reply to your question about retaining the repeat-counting check if timeout is not specified:
when I was testing the PR code, I found that specifying the timeout as zero (hence having the function ignore timeout events) was quite useful. I could physically disconnect the IMU from the serial cable, and then reconnect it, and your library would continue working nicely and ROS messages would start again. I can see situations where for reliability, the user would prefer code that can tolerate interruptions and continues once the problem goes away, rather than exit with an error.

I did include a default timeout value of 1000 ms, which is probably too long for most people. It can certainly be changed to something smaller (like 100 ms).

One alternative may be to set a default timeout period to 3 times the polling interval? Then, if the user does not specify timeout explicitly, use this default value. This may be a bit messy to implement though.

[gsokoll]

Have made changes to check for a user-defined timeout. If no timeout defined, then defaults to timeout equal to 3 times the polling interval which should replicate the previous repeat-count check approach.

[twdragon]

@thank you a lot! Great contribution! I will merge the PR very soon