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

[WIP] Continue development and upstream #94

Closed
wants to merge 24 commits into from
Closed

[WIP] Continue development and upstream #94

wants to merge 24 commits into from

Conversation

hoffmann-stefan
Copy link
Member

@hoffmann-stefan hoffmann-stefan commented Jun 15, 2022 •
edited

from #49 (comment):

Though I have to be honest, depending on when this will be finished (or at least
in a somewhat usable state compatible to the other APIs in #90) we can use this.
On the other hand if this isn't ready when we need the features in our internal
code I may not have a choice but implement this myself in order to avoid
delaying the development of our internal stuff/products.

[...]

In the case that I implement this for our internal products I would continue
sharing the code in the open and finding a way to contribute this back or
helping integrating your/other implementations into the main branch, including
updating our internal code afterwards to be on the main branch of this project
once actions are merged (with all the qualities described in my first post in
this issue).

Now we are at the state where we need action support next.
As described above we would like to contribute to the open and avoid doing a hard fork.
This PR is a grab bag of what is comming next on our development roadmap for this library.
I avoid to do multiple PRs that depend on each other for now, those will be done one the main branch moves forward. I will be using prefixes in the commit messages how those would be grouped to seperate PRs so this should be easyer later on.

Current roadmap:

  • Fix some warnings (so other warnings don't get lost while developing).
  • Use dotnet code style (so I don't constantly fight my habbits and editor configuration).
  • Add support for GuardConditions.
  • Add support for SynchronizationContext to make async code easier. -> Add support for SynchronizationContext #112
  • Add support for action service and client. Support for Actions #49
    • IDL message generation.
    • Add basic action client
    • Add basic action service
    • Testing.

cc @esteve @ooeygui @shschaefer

@hoffmann-stefan hoffmann-stefan marked this pull request as draft June 15, 2022 14:31
This was referenced Jun 24, 2022
Closed
Closed
@pabloinigoblasco
Copy link

Hello. What is the state of this development now?
Is it something we could try?

@hoffmann-stefan
Copy link
Member Author

@pabloinigoblasco

This is work in progress, though we use this internally for our products as we need the features right now.
Should be fine to checkout and test and give feedback.
Though I may do some force-pushes here as I develop this further, and there might be breaking changes as well. But better then nothing if you need the features. Maybe use a copy of this branch that you can update independently of me pushing things here.

@esteve stated some days ago that he wants to look at my other PRs (that this is based on) and likely merge them to main: #90 (comment)

This was referenced Apr 22, 2023
Closed
Closed
@hoffmann-stefan hoffmann-stefan force-pushed the continue-develop-and-upstream branch 2 times, most recently from 0a16152 to 6292527 Compare April 22, 2023 11:37
This was referenced Apr 22, 2023
Merged
Merged
@esteve esteve deleted the branch ros2-dotnet:main May 2, 2023 13:25
@esteve esteve closed this May 2, 2023
@hoffmann-stefan hoffmann-stefan changed the base branch from master to main May 2, 2023 17:10
@hoffmann-stefan hoffmann-stefan mentioned this pull request May 15, 2023
Merged
@hoffmann-stefan
actions: initial action idl generation
5512e7e
@hoffmann-stefan
actions: extend idl generation to include interfaces for "action wrap…
e146a1f 
…per types"

With "action wrapper types" this means the ROS messages and services that add the `GoalId`, `TimeStamp` or other fields on top of the user defined types for the action.
The introduced methods in `IRosActionDefinition` are needed to work around the missing existential type support in C# (dotnet/csharplang#5556).
@hoffmann-stefan
actions: add accessors to UUID and Time properties for "action wr…
811170e 
…apper types"

This uses the base type `IMessage` to get around the cyclic references.
@hoffmann-stefan
actions: add ActionDefinitionStaticMemberCache
78b39d1
@hoffmann-stefan
actions: add public action client API without implementation
fcc3cb1
@hoffmann-stefan
actions: rename ServiceIsReady() to ServerIsReady() and move to b…
db7f9a0 
…ase class
@hoffmann-stefan
actions: add helper for converting guid <-> uuid
e672ca1
@hoffmann-stefan
actions: implement basic action client
d7bd8b6
@hoffmann-stefan
actions: add public action server API without implementation
526deb3
@hoffmann-stefan
actions: add missing license header
b3f0fda
@hoffmann-stefan
actions: Change IsCancelRequested to IsCanceling, add IsExecuting
98faadd
@hoffmann-stefan
actions: Add ReadFromMessageHandle/WriteToMessageHandle helper mehtods
705e8bb
@hoffmann-stefan
actions: implement basic action server
8f5ce01
@hoffmann-stefan
actions: add example action client and server
7ee301e
@hoffmann-stefan
actions: make local variable name more obvious
25e04a8
@hoffmann-stefan
actions: use return value from Interlocked.CompareExchange
1098b88 
The field access after Interlocked.CompareExchange might not reflect the updated value.
Or does it? Anyways this makes it more clear.
@hoffmann-stefan
actions: reorder HandleGoalResponse, HandleStatusMessage and HandleFe…
3552b8f 
…edbackMessage
@hoffmann-stefan
actions: handle action specific take failed error codes
e18b37a
@hoffmann-stefan
actions: publish status before sending the result response
5bada3b
@hoffmann-stefan
actions: fix typo in dependency
23b1455
@hoffmann-stefan
bool pInvoke: Fix bool pInvokes in rcldotnet
419e0df
@hoffmann-stefan
bool pInvoke: Fix bool pInvokes in generated messages
7c04354 
This does not change the C# side of the generated messages to avoid even more complexity in the templates.
As bools in C# pInvokes get marshalled as 32 bit integer by default there is no strictly need to change the code for bool fields.
@hoffmann-stefan
qos: add quality of service for publishers and subscribers.
dfda4e0 
Most of the documentation was taken from ros2_rust. https://github.com/ros2-rust/ros2_rust/blob/main/rclrs/src/qos.rs
@hoffmann-stefan
qos: fix enum casts for humble (and maybe galactic)
677aa1a
This was referenced May 19, 2023
Merged
Merged
Merged
@hoffmann-stefan
Copy link
Member Author

Closing this PR as all parts where now extracted into seperate PRs and only one PR is missing to be merged. So this PR dosn't provide any more use (for now it was the PR with all not merged changes integrated), but as only one is left to merge #108 can be used instead.

This is the list of all PRs that came out of this:

@hoffmann-stefan hoffmann-stefan deleted the continue-develop-and-upstream branch May 29, 2023 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hoffmann-stefan @pabloinigoblasco @esteve