Amanda/read chunks #62
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mpinol
reviewed
Feb 5, 2021
com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs
Outdated
Show resolved
Hide resolved
k size
com.unity.robotics.ros-tcp-connector/Editor/ROSSettingsEditor.cs
Outdated
Show resolved
Hide resolved
com.unity.robotics.ros-tcp-connector/Runtime/TcpConnector/ROSConnection.cs
Outdated
Show resolved
Hide resolved
Receiving large messages like high(ish)-res image streams was causing the |
|
Ok, and you've tested this version on some workloads that are likely to make it retry at least a few times, right? |
Works on at least messages of byte length at least < 264033 or so, confirming that it retries between 0-3 attempts in stream without losing images or showing partial images. |
LaurieCheers-unity
approved these changes
Feb 8, 2021
LaurieCheers-unity
added a commit
that referenced
this pull request
Apr 21, 2021
* Add ROS# link (#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (#61) * Amanda/read chunks (#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (#64) * M prefix for all messages (#66) * Topic list service (#72) * Adding install instructions to README Partially addressing issue #71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (#87) * Update version number in readme, and clean up some weird instructions (#89) Co-authored-by: mpinol <[email protected]> Co-authored-by: flonou <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]>
sdiao
added a commit
that referenced
this pull request
May 27, 2021
* Merge 0.3.0 to main (#90) * Add ROS# link (#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (#61) * Amanda/read chunks (#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (#64) * M prefix for all messages (#66) * Topic list service (#72) * Adding install instructions to README Partially addressing issue #71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (#87) * Update version number in readme, and clean up some weird instructions (#89) Co-authored-by: mpinol <[email protected]> Co-authored-by: flonou <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * Remove deprecated files Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: mpinol <[email protected]> Co-authored-by: flonou <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]>
sdiao
added a commit
that referenced
this pull request
Jun 1, 2021
* Add ROS# link (#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (#61) * Amanda/read chunks (#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (#64) * M prefix for all messages (#66) * Topic list service (#72) * Adding install instructions to README Partially addressing issue #71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (#87) * Update version number in readme, and clean up some weird instructions (#89) * MDuration namespace change (#91) * Minor link fix (#98) * RosConnection 2.0 (#94) * add changelog (#100) add changelog * Forum links to README, config.yml (#101) * Laurie/connection status hud (#99) * add test coverage (#102) * Reducing character count for path to generated messages The path from the package root to the root of the PregeneratedMessages folder was 47 characters. From within the PickAndPlace project, the fully qualified path including drive letter, path to workspace, and path to its location in the package cache can easily exceed 200 characters, especially for users with long names or deep workspace organization. The maximum allowed path length for a fully qualified path in Windows is 260 characters, after which an IDE can not compile the assemblies for source with too-long paths. Moved Runtime/MessageGeneration/PregeneratedMessages to Runtime/Messages, which saves 30 characters off the full path name. This is a mitigation that can not fully solve the problem, but wil at least reduce the number of cases for which the ROS-TCP-Endpoint assemblies fail to compile due to path length. Tested on my machine by manually renaming the directory in my PackageCache. * Add Linter (#103) * Add editorconfig for formatting * Use linter to format the package code * Add pre-commit hook * Fix line endings for markdown Fix trailing-whitespace * Remove ruby setup; Change to run pre-commit only on pull-requests * Resolve conflicts (#106) * Shuo/resolve conflict (#107) * Merge 0.3.0 to main (#90) * Add ROS# link (#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (#61) * Amanda/read chunks (#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (#64) * M prefix for all messages (#66) * Topic list service (#72) * Adding install instructions to README Partially addressing issue #71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (#87) * Update version number in readme, and clean up some weird instructions (#89) Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * Remove deprecated files Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * add changelog meta file (#109) * update version to 0.4.0 (#110) * rremove stale merge Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: peifeng-unity <[email protected]>
sdiao
added a commit
that referenced
this pull request
Jun 1, 2021
* Add ROS# link (#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (#61) * Amanda/read chunks (#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (#64) * M prefix for all messages (#66) * Topic list service (#72) * Adding install instructions to README Partially addressing issue #71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (#87) * Update version number in readme, and clean up some weird instructions (#89) * MDuration namespace change (#91) * Minor link fix (#98) * RosConnection 2.0 (#94) * add changelog (#100) add changelog * Forum links to README, config.yml (#101) * Laurie/connection status hud (#99) * add test coverage (#102) * Reducing character count for path to generated messages The path from the package root to the root of the PregeneratedMessages folder was 47 characters. From within the PickAndPlace project, the fully qualified path including drive letter, path to workspace, and path to its location in the package cache can easily exceed 200 characters, especially for users with long names or deep workspace organization. The maximum allowed path length for a fully qualified path in Windows is 260 characters, after which an IDE can not compile the assemblies for source with too-long paths. Moved Runtime/MessageGeneration/PregeneratedMessages to Runtime/Messages, which saves 30 characters off the full path name. This is a mitigation that can not fully solve the problem, but wil at least reduce the number of cases for which the ROS-TCP-Endpoint assemblies fail to compile due to path length. Tested on my machine by manually renaming the directory in my PackageCache. * Add Linter (#103) * Add editorconfig for formatting * Use linter to format the package code * Add pre-commit hook * Fix line endings for markdown Fix trailing-whitespace * Remove ruby setup; Change to run pre-commit only on pull-requests * Resolve conflicts (#106) * Shuo/resolve conflict (#107) * Merge 0.3.0 to main (#90) * Add ROS# link (#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (#61) * Amanda/read chunks (#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (#64) * M prefix for all messages (#66) * Topic list service (#72) * Adding install instructions to README Partially addressing issue #71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (#87) * Update version number in readme, and clean up some weird instructions (#89) Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * Remove deprecated files Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * add changelog meta file (#109) * update version to 0.4.0 (#110) * rremove stale merge Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: peifeng-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: peifeng-unity <[email protected]>
P3TE
pushed a commit
to P3TE/ROS-TCP-Connector
that referenced
this pull request
Jul 13, 2021
* Add ROS# link (Unity-Technologies#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (Unity-Technologies#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (Unity-Technologies#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (Unity-Technologies#61) * Amanda/read chunks (Unity-Technologies#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (Unity-Technologies#64) * M prefix for all messages (Unity-Technologies#66) * Topic list service (Unity-Technologies#72) * Adding install instructions to README Partially addressing issue Unity-Technologies#71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (Unity-Technologies#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (Unity-Technologies#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (Unity-Technologies#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (Unity-Technologies#87) * Update version number in readme, and clean up some weird instructions (Unity-Technologies#89) * MDuration namespace change (Unity-Technologies#91) * Minor link fix (Unity-Technologies#98) * RosConnection 2.0 (Unity-Technologies#94) * add changelog (Unity-Technologies#100) add changelog * Forum links to README, config.yml (Unity-Technologies#101) * Laurie/connection status hud (Unity-Technologies#99) * add test coverage (Unity-Technologies#102) * Reducing character count for path to generated messages The path from the package root to the root of the PregeneratedMessages folder was 47 characters. From within the PickAndPlace project, the fully qualified path including drive letter, path to workspace, and path to its location in the package cache can easily exceed 200 characters, especially for users with long names or deep workspace organization. The maximum allowed path length for a fully qualified path in Windows is 260 characters, after which an IDE can not compile the assemblies for source with too-long paths. Moved Runtime/MessageGeneration/PregeneratedMessages to Runtime/Messages, which saves 30 characters off the full path name. This is a mitigation that can not fully solve the problem, but wil at least reduce the number of cases for which the ROS-TCP-Endpoint assemblies fail to compile due to path length. Tested on my machine by manually renaming the directory in my PackageCache. * Add Linter (Unity-Technologies#103) * Add editorconfig for formatting * Use linter to format the package code * Add pre-commit hook * Fix line endings for markdown Fix trailing-whitespace * Remove ruby setup; Change to run pre-commit only on pull-requests * Resolve conflicts (Unity-Technologies#106) * Shuo/resolve conflict (Unity-Technologies#107) * Merge 0.3.0 to main (Unity-Technologies#90) * Add ROS# link (Unity-Technologies#55) * Add ROS# link * Fixing message name that should not take a capital first letter automatically (Unity-Technologies#47) * Fixing message name that should not take a capital first letter automatically * Adding warning when a message file name starts with a lowercase character * AIRO-331 Code Cleanup and Testing Framework (Unity-Technologies#57) * Moving package files into package subdirectory * Adding tests and fixing Yamato config. * Adding missing using statement to MessageParser * RosMessageGeneration namespace change (Unity-Technologies#61) * Amanda/read chunks (Unity-Technologies#62) * Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback * Adding prebuilt ObjectRecognition and Octomap messages (Unity-Technologies#64) * M prefix for all messages (Unity-Technologies#66) * Topic list service (Unity-Technologies#72) * Adding install instructions to README Partially addressing issue Unity-Technologies#71 wherein a user couldn't find up-to-date instructions for installing the connector into a new project. * Updating Contribution Content (Unity-Technologies#77) * Contributing content commit * Link fix * PR feedback * Adding PR template (Unity-Technologies#81) * One persistent connection for all messages sent to Unity * Read multiple messages from each connection * Compiles * First thing sent on each new connection is the timeout * Comments! * Stop the connection on leaving play mode * Add a "timeout on idle" config setting so Endpoint doesn't have to send it * IP address check for Unity IP address and ROS IP address (Unity-Technologies#80) * Add Ip address check * Adding condition for alphaumeric IP address https://man7.org/linux/man-pages/man5/hosts.5.html * Resolve conflicts Resolve the conflicts to merge dev to main * Update package.json (Unity-Technologies#87) * Update version number in readme, and clean up some weird instructions (Unity-Technologies#89) Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * Remove deprecated files Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> * add changelog meta file (Unity-Technologies#109) * update version to 0.4.0 (Unity-Technologies#110) * rremove stale merge Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: LaurieCheers-unity <[email protected]> Co-authored-by: Amanda <[email protected]> Co-authored-by: vidurvij-Unity <[email protected]> Co-authored-by: peifeng-unity <[email protected]> Co-authored-by: Devin Miller (Unity) <[email protected]> Co-authored-by: peifeng-unity <[email protected]>
peifeng-unity
pushed a commit
that referenced
this pull request
Jul 15, 2021
* Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback
peifeng-unity
pushed a commit
that referenced
this pull request
Jul 15, 2021
* Read message in chunks min * Removing unpredictable DataAvailable check * Configurable byte chun k size * Prevent infinite hang on reading message contents * Wait for frame on data read retry * PR Feedback
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Re: Unity-Technologies/Unity-Robotics-Hub#101
networkStream.DataAvailabledoesn't ensure that the sender has finished sending everything, just that there's nothing left in the read buffer, afaik. This was causing reads to end unexpectedly early for larger messages such as images.