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

microRTPS: fix UART link #15354

Merged
merged 15 commits into from
Jul 31, 2020
Merged

microRTPS: fix UART link #15354

merged 15 commits into from
Jul 31, 2020

Conversation

TSC21
Copy link
Member

@TSC21 TSC21 commented Jul 16, 2020

Currently the microRTPS bridge UART link is broken and it's resulting on continuous CRC errors on the agent side that after some seconds close the link. This is being investigated.

Also, it was verified that a stack increase was required to avoid the micrortps_client of causing and hardfault and rebooting the flight controller.

@TSC21 TSC21 force-pushed the pr-micrortps_client_various_fixes branch 4 times, most recently from edfd098 to 6fcfbb0 Compare July 18, 2020 08:43
@TSC21 TSC21 force-pushed the pr-micrortps_client_various_fixes branch from 13dfd34 to 43b1152 Compare July 20, 2020 09:39
@TSC21
Copy link
Member Author

TSC21 commented Jul 20, 2020

@AndrewWedemier thanks for your insights and your debugging effort, which ended-up helping me unblock this! Added you as a co-author in one commit, which was the commit that allows to have the bridge usable over UART.

Note that for setups which are to run at baud rates <=1MByte/sec, it's required to throttle the overall publishing using the -w option. So with the current default streams (per 07/20/2020), something like:

micrortps_client start -d /dev/ttyS2 -b 1000000 -v -w 6
./micrortps_agent -d /dev/ttyUSB0 -b 1000000 -v -w 6

works nice without any errors, though it introduces some latency on the link. The alternative is also increasing the baud rate. Found that using 1.5MByte/s with the current default streams is enough to have it work properly.

micrortps_client start -d /dev/ttyS2 -b 1500000 -v
./micrortps_agent -d /dev/ttyUSB0 -b 1500000 -v

Note that with the increase of the number of topics being streamed, and depending on each rates, this might change and might require changes on the stream rates.

@TSC21 TSC21 changed the title [WIP]: fix microRTPS bridge UART link microRTPS: fix UART link Jul 20, 2020
@TSC21 TSC21 marked this pull request as ready for review July 20, 2020 14:16
@TSC21 TSC21 requested a review from bkueng July 20, 2020 14:16
msg/templates/uorb_microcdr/microRTPS_client.cpp.em Outdated Show resolved Hide resolved
msg/templates/uorb_microcdr/microRTPS_client.cpp.em Outdated Show resolved Hide resolved
msg/templates/urtps/microRTPS_agent.cpp.em Outdated Show resolved Hide resolved
msg/templates/urtps/microRTPS_transport.h Outdated Show resolved Hide resolved
msg/templates/urtps/microRTPS_transport.cpp Outdated Show resolved Hide resolved
msg/templates/urtps/microRTPS_transport.cpp Outdated Show resolved Hide resolved
@TSC21 TSC21 force-pushed the pr-micrortps_client_various_fixes branch 2 times, most recently from a7112c4 to 2b6fda8 Compare July 28, 2020 18:02
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better, thanks.
Can you also cleanup the commits? There should not be revert commits.

msg/templates/uorb_microcdr/microRTPS_client.cpp.em Outdated Show resolved Hide resolved
msg/templates/uorb_microcdr/microRTPS_client.cpp.em Outdated Show resolved Hide resolved
msg/templates/uorb_microcdr/microRTPS_client.cpp.em Outdated Show resolved Hide resolved
msg/templates/urtps/microRTPS_transport.cpp Outdated Show resolved Hide resolved
msg/templates/urtps/microRTPS_transport.cpp Show resolved Hide resolved
@TSC21 TSC21 force-pushed the pr-micrortps_client_various_fixes branch from 2b6fda8 to 46d20df Compare July 29, 2020 13:11
@TSC21 TSC21 force-pushed the pr-micrortps_client_various_fixes branch from 46d20df to 548a3aa Compare July 29, 2020 13:30
@TSC21 TSC21 merged commit 342b1c5 into master Jul 31, 2020
@TSC21 TSC21 deleted the pr-micrortps_client_various_fixes branch July 31, 2020 13:13
@TSC21
Copy link
Member Author

TSC21 commented Jul 31, 2020

Thanks @bkueng!

@TSC21 TSC21 added this to the Release v1.11.0 milestone Jul 31, 2020
@mrpollo mrpollo mentioned this pull request Aug 19, 2020
@Jaeyoung-Lim Jaeyoung-Lim restored the pr-micrortps_client_various_fixes branch August 24, 2020 12:07
@Jaeyoung-Lim Jaeyoung-Lim deleted the pr-micrortps_client_various_fixes branch August 24, 2020 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants