Skip to content

adding connection_status#14

Merged
philbucher merged 2 commits into
masterfrom
add-connection-status
May 8, 2020
Merged

adding connection_status#14
philbucher merged 2 commits into
masterfrom
add-connection-status

Conversation

@philbucher

Copy link
Copy Markdown
Member

now the connection_status should be working as you describe in the tutorial

@philbucher philbucher requested a review from pooyan-dadvand May 8, 2020 13:10
Comment thread co_sim_io/impl/connection.hpp Outdated
return ReturnInfo(); // TODO use this, should put a return code based on the bool that comes back from mpComm->Connect()
const bool is_connected = mpComm->Connect();
ReturnInfo ret_info;
ret_info.Set<bool>("connection_status", is_connected);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought that you had an int with different options.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

sure, changed it 🤦‍♂️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would also add some constexpr status values like:

  • NotConnected: Connect is not called yet
  • Connected: after successful call to Connect
  • Disconnected: after successful call to Disconnect
  • ConnectionError : Having a error message in return_info
  • DisconnectionError: Having an error message in return_info

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How would this work for C?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

enums, or const int values

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Wouldn't it then make more sense to use enums also in C++?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As you like, there is an implicit conversion from enum to int in both languages

@philbucher philbucher merged commit 9b5521f into master May 8, 2020
@philbucher philbucher deleted the add-connection-status branch May 8, 2020 13:34
This was referenced May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants