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

Feature/csharp folder structure 2 #1616

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

KerstinKeller
Copy link
Contributor

Description

This changes the folder layout of the C# eCAL bindings.
They can be build 100% standalone against an eCAL installation.

All C# related code is not in the subfolder `lang/csharp`.
The folders are then separated based on the functionality.
All related samples share the same CMakeLists.txt file, which makes it easier to add new samples.
…f.Person.Samples.Datatypes

- Calculate C# protobuf version from available protoc.
@KerstinKeller KerstinKeller added the cherry-pick-to-NONE Don't cherry-pick these changes label May 28, 2024
set(minimal_rec_cs_src
minimal_rec.cs
)
macro(ecal_add_csharp_core_sample sample_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move this macro to /cmake/helper_functions/ecal_add_functions.cmake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to have any dependencies to the "outside", e.g. lang/csharp should be standalone...

{
string message = "HELLO WORLD FROM C#";
publisher.Send(message, -1);
var received_message = subscriber.Receive(100);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are publisher/subscriber are registered fast enough to establish a connection and exchange the test payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the "sleep" is done beforehand.


set_target_properties(${PROJECT_NAME} PROPERTIES
VS_GLOBAL_ROOTNAMESPACE ${PROJECT_NAME}
macro(ecal_add_csharp_protobuf_sample sample_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move this macro to /cmake/helper_functions/ecal_add_functions.cmake


set_target_properties(${PROJECT_NAME} PROPERTIES
VS_GLOBAL_ROOTNAMESPACE ${PROJECT_NAME}
VS_DOTNET_TARGET_FRAMEWORK_VERSION "v4.6.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

can the installed dot net framework version be dectected programmatically instead of hard-coded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we can think about making it a CMake variable. But i would do it in a separate PR, in needs to be modified in a few places.


{
publisher.Send(person0);
var person_rec_0 = subscriber.Receive(100);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here with the connection timespan between pub and sub.

Copy link
Contributor

@hannemn hannemn left a comment

Choose a reason for hiding this comment

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

No major findings - just a few cosmetic things. ;-)

@KerstinKeller KerstinKeller merged commit f16f46a into master Jun 18, 2024
17 of 19 checks passed
@KerstinKeller KerstinKeller deleted the feature/csharp-folder-structure-2 branch June 21, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-NONE Don't cherry-pick these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants