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.

Sign up for GitHub

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

Jump to bottom

Refactor external transports #179

Merged
merged 26 commits into from
Feb 9, 2021
Merged

Refactor external transports #179

merged 26 commits into from
Feb 9, 2021

Conversation

pablogs9
Copy link
Member

@pablogs9 pablogs9 commented Dec 17, 2020 •
edited
Loading

This PR:

  • Isolates the XRCE framing code to be used in any transport
  • Renames the framing code from SerialIO to FramingIO
  • Fixes the issue that does not take into account the number of written/read bytes in SerialIO (Fix SerialIO small transports payloads #178)
  • Adds a new profile for Custom transports where the user can set open/close/read/write callbacks for transports at runtime. These Custom transports can be configured to use FramingIO.
  • Removes the old approach for adding custom transports Marks as deprecated the old approach for adding custom transports. This should be removed in the next major version.

TODO before merge:

Related:

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@pablogs9 pablogs9 mentioned this pull request Dec 17, 2020
Closed
1 task
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@pablogs9 pablogs9 force-pushed the feature/refactor_external_transports branch from c5414b3 to 55cf00a Compare December 17, 2020 08:34
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@pablogs9 pablogs9 force-pushed the feature/refactor_external_transports branch from 55cf00a to 4537cb4 Compare December 17, 2020 08:57
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@pablogs9 pablogs9 force-pushed the feature/refactor_external_transports branch from 4537cb4 to 36bbeeb Compare December 17, 2020 09:36
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@pablogs9 pablogs9 mentioned this pull request Jan 21, 2021
Closed
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

This was referenced Jan 26, 2021
Merged
Merged
Merged
Merged
Merged
@pablogs9 pablogs9 force-pushed the feature/refactor_external_transports branch from 6196b29 to b03ecec Compare February 2, 2021 08:51
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

jamoralp
jamoralp reviewed Feb 2, 2021
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
test/transport/custom_comm/CustomComm.cpp Outdated Show resolved Hide resolved
test/transport/custom_comm/CustomComm.hpp Outdated Show resolved Hide resolved
test/transport/custom_comm/CustomComm.hpp Outdated Show resolved Hide resolved
test/transport/custom_comm/CustomComm.hpp Outdated Show resolved Hide resolved
test/transport/custom_comm/CustomComm.hpp Show resolved Hide resolved
test/transport/custom_comm/CustomComm.cpp Outdated Show resolved Hide resolved
test/transport/custom_comm/CustomComm.cpp Outdated Show resolved Hide resolved
@pablogs9 pablogs9 mentioned this pull request Feb 3, 2021
Merged
@pablogs9 pablogs9 marked this pull request as ready for review February 3, 2021 07:03
@pablogs9 pablogs9 requested a review from jamoralp February 3, 2021 07:03
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

jamoralp
jamoralp previously approved these changes Feb 4, 2021
View reviewed changes
Copy link
Contributor

@jamoralp jamoralp left a comment

Choose a reason for hiding this comment

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

LGTM, ready to merge 👍

pablogs9 and others added 22 commits February 9, 2021 09:10
@pablogs9 @jamoralp
Move framing to runtime
33865e5 
Update custom callbacks API


Fix


Minor fixes
@pablogs9 @jamoralp
Updates
d3eab69
@pablogs9 @jamoralp
Add test
cde77fc
@pablogs9 @jamoralp
Update test
575e2d0
@pablogs9 @jamoralp
Fix Ralph issue
34a32f0
@pablogs9 @jamoralp
Remove old external transport approach
8f11406
@pablogs9 @jamoralp
Revert "Remove old external transport approach"
609bfc9 
This reverts commit dbd94b6.
@pablogs9 @jamoralp
Add deprecation message
72863cf
@pablogs9 @jamoralp
Fix leak in test
d2c5ae4 
Fix leak in test


Fix


Fix test
@pablogs9 @jamoralp
Fix windows test
ab4714b
@pablogs9 @jamoralp
Minor fix
889ba95
@pablogs9 @jamoralp
Update
7f9e074
@pablogs9 @jamoralp
Remove dead code
03fb7e4
@pablogs9 @jamoralp
Remove old transports approach
98444ea
@pablogs9 @jamoralp
Address comments
9cb6c69
@pablogs9 @jamoralp
Update test/transport/custom_comm/CustomComm.cpp
24f404e 
Co-authored-by: Jose Antonio Moral <[email protected]>
@pablogs9 @jamoralp
Update test/transport/custom_comm/CustomComm.hpp
e987c74 
Co-authored-by: Jose Antonio Moral <[email protected]>
@pablogs9 @jamoralp
Update test/transport/custom_comm/CustomComm.hpp
0a2b078 
Co-authored-by: Jose Antonio Moral <[email protected]>
@pablogs9 @jamoralp
Update test/transport/custom_comm/CustomComm.hpp
22eff61 
Co-authored-by: Jose Antonio Moral <[email protected]>
@pablogs9 @jamoralp
Address comments
51cb17f
@pablogs9 @jamoralp
Address comments
8a3ea46
@pablogs9 @jamoralp
Address comments
7ea48d0
@jamoralp jamoralp force-pushed the feature/refactor_external_transports branch from ad59caf to 7ea48d0 Compare February 9, 2021 08:11
@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Windows Build Status

@pablogs9 pablogs9 requested a review from jamoralp February 9, 2021 10:24
@jamoralp jamoralp merged commit 22bff80 into develop Feb 9, 2021
@jamoralp jamoralp deleted the feature/refactor_external_transports branch February 9, 2021 10:28
@pablogs9 pablogs9 mentioned this pull request Feb 10, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamoralp jamoralp jamoralp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pablogs9 @richiware @jamoralp