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

Improve primary client #137

Closed

Conversation

urmarp
Copy link
Contributor

@urmarp urmarp commented Dec 28, 2022

Improved primary interface, this includes following changes:

  • Added message handlers for messages received through primary client

  • Changed primary pipeline and full driver example

  • Added datatype: vector6f_t and vector6uin8_t

  • Added parsing of vector6f_t

  • Changed timeout in producer

  • Added parsing of more messages from primary interface

  • Added calibration check in constructor of primary client

  • Added calibration checksum to urdriver constructor

  • Added test for primary client and primary parser

  • Added funtionality to set robot as simulated or real

  • Added wait in full driver example

@codecov
Copy link

codecov bot commented Dec 28, 2022

Codecov Report

Base: 74.09% // Head: 75.53% // Increases project coverage by +1.44% 🎉

Coverage data is based on head (1d13676) compared to base (09adab9).
Patch coverage: 68.91% of modified lines in pull request are covered.

❗ Current head 1d13676 differs from pull request most recent head 3cd736a. Consider uploading reports for the commit 3cd736a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #137      +/-   ##
==========================================
+ Coverage   74.09%   75.53%   +1.44%     
==========================================
  Files          91      135      +44     
  Lines        3462     4775    +1313     
  Branches      359      429      +70     
==========================================
+ Hits         2565     3607    +1042     
- Misses        680      931     +251     
- Partials      217      237      +20     
Impacted Files Coverage Δ
include/ur_client_library/comm/bin_parser.h 89.23% <ø> (+14.76%) ⬆️
...client_library/primary/abstract_primary_consumer.h 100.00% <ø> (ø)
.../ur_client_library/primary/program_state_message.h 0.00% <0.00%> (ø)
include/ur_client_library/primary/robot_message.h 100.00% <ø> (ø)
.../primary/robot_message/runtime_exception_message.h 0.00% <0.00%> (ø)
include/ur_client_library/primary/robot_state.h 100.00% <ø> (ø)
include/ur_client_library/ur/calibration_checker.h 83.33% <0.00%> (+16.66%) ⬆️
include/ur_client_library/ur/ur_driver.h 33.33% <ø> (ø)
src/primary/program_state_message.cpp 0.00% <0.00%> (ø)
...rimary/robot_message/runtime_exception_message.cpp 0.00% <0.00%> (ø)
... and 100 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@fmauch
Copy link
Collaborator

fmauch commented Jan 5, 2023

@urmarp this would be easier to review if these were multiple commits.

@fmauch fmauch self-requested a review January 5, 2023 10:54
Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Before reviewing this any further: Could you please elaborate what you have done? What is the handler concept and why did you add this?

Again, splitting this into multiple commits (maybe with commit messages going into more detail) might make things a lot clearer.

Comment on lines 42 to 43
// const std::string DEFAULT_ROBOT_IP = "192.168.56.101";
const std::string DEFAULT_ROBOT_IP = "localhost";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not change such a thing without a reason. The IP can be specified as an argument and we designed the example in order to work with the start_ursim.sh script.

Comment on lines 118 to 119
// Once RTDE communication is started, we have to make sure to read from the interface buffer, as
// otherwise we will get pipeline overflows. Therefor, do this directly before starting your main
// otherwise we will get pipeline overflows. Therefore, do this directly before starting your main
// loop.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment belongs to the g_my_driver->startRTDECommunication(); line.

Comment on lines 37 to 38
// const std::string DEFAULT_ROBOT_IP = "192.168.56.101";
const std::string DEFAULT_ROBOT_IP = "localhost";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above. Please do not change this.

include/ur_client_library/comm/producer.h Show resolved Hide resolved
…se messages for primary client joint data messages.
…hese handlers save the data from the latest message of each type, which can then be accessed through the primary client. The handlers also enable the possibility to handle the data differently for each message.
… the kinematics info message type and a calibration checker. It is still possible to add a separate calibration checker, if that is desired.
…umer get the message handlers and from the message handlers get the latest message of each type.

The primary client checks whether the robot is in remote control or not, as it should not be able to send script if the robot is not in remote control. This is not true if the robot is simulated. Therefore, when constructing a primary client instance it is needed to specify if the robot is simulated or real. I was not able to find a way for the primary client to check whether it was real or simulated automatically.

The primary client now automatically checks whether the checksum is correct when starting.
…iver now also checks if the checksum is correct when an instance is made, as it makes an instance of the primary client, which checks when it is instantiated.

Also added debugging message to tcp socket, which specifies which socket is already connected.
@urmarp urmarp force-pushed the improve_primary_interface_v2 branch from 1f26501 to 1d13676 Compare January 6, 2023 08:30
@urmarp
Copy link
Contributor Author

urmarp commented Jan 6, 2023

Before reviewing this any further: Could you please elaborate what you have done? What is the handler concept and why did you add this?

Again, splitting this into multiple commits (maybe with commit messages going into more detail) might make things a lot clearer.

I have split the commit into multiple commits and tried to explain the thoughts behind the changes.

Added message handlers for the messages from the primary interface. These handlers save the data from the latest message of each type, which can then be accessed through the primary client. The handlers also enable the possibility to handle the data differently for each message. The primary client can through its consumer get the message handlers and from the message handlers get the latest message of each type.

…trol_ will always be true. The thread to check if robot is in remote control or not will only start if robot is e-series.
…ity to determine if robot is e-series or cb3.
@fmauch
Copy link
Collaborator

fmauch commented May 28, 2024

Closing this in favor of #186

@fmauch fmauch closed this May 28, 2024
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.

None yet

2 participants