Polyscopex dashboard client #392
Conversation
Add PolyScope X dashboard client aka Robot API to the client library.
urfeex
force-pushed
the
polyscopex_dashboard_client
branch
from
c661f42 to
5291e9b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #392 +/- ##
==========================================
+ Coverage 77.00% 82.89% +5.89%
==========================================
Files 92 1 -91
Lines 4287 269 -4018
Branches 482 0 -482
==========================================
- Hits 3301 223 -3078
+ Misses 743 46 -697
+ Partials 243 0 -243
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
urfeex
force-pushed
the
polyscopex_dashboard_client
branch
from
5291e9b to
dc018dc
Compare
urfeex
force-pushed
the
polyscopex_dashboard_client
branch
from
30f9749 to
0820448
Compare
|
|
||
| if (!my_dashboard->commandPowerOff()) | ||
| // Bring the robot to a defined state being powered off. We're ignoring errors here since | ||
| // powering off an already powered off robot will return an error. |
There was a problem hiding this comment.
This comment doesn't make sense as we return 1, if we fail to commandPowerOff, at least for generation 5.
There was a problem hiding this comment.
Right, I should update it. The comment actually only applies for PolyScope X
| URCL_LOG_ERROR("Could not play program"); | ||
| return 1; | ||
| // For PolyScope X, the command is called "resume" | ||
| if (!my_dashboard->commandResume()) |
There was a problem hiding this comment.
Will this work, isn't this command for when the program is paused?
There was a problem hiding this comment.
The program should be paused at this stage.
| * be made successfully. | ||
| * | ||
| */ | ||
| DashboardResponse commandPowerOnWithResponse(const std::chrono::duration<double> timeout = std::chrono::seconds(300)); |
There was a problem hiding this comment.
300 seconds seems like a lot
| /*! | ||
| * \brief Send Power off command | ||
| * | ||
| * \return The response string returned by the server |
There was a problem hiding this comment.
Isn't it returning an object and not a string.
Should we consider removing return and throw in the description, as this is not included for all other of the commands with response, to make it consistent
| bool commandIsInRemoteControl(); | ||
|
|
||
| DashboardResponse commandIsInRemoteControlWithResponse(); | ||
| /*! |
There was a problem hiding this comment.
Move description above command
| throw NotImplementedException("sendAndReceive is not implemented for DashboardClientImplX."); | ||
| } | ||
|
|
||
| bool DashboardClientImplX::connect(const size_t max_num_tries, const std::chrono::milliseconds reconnection_time) |
There was a problem hiding this comment.
Should we add a notice with a log a message to this function that it is not really available for PolyscopeX as there is nothing to connect to?
| return false; | ||
| } | ||
|
|
||
| void DashboardClientImplX::disconnect() |
There was a problem hiding this comment.
Same as for connect, some sort of notice?
| bool DashboardClientImplX::sendRequest(const std::string& command_str, const std::string& expected_response_pattern, | ||
| const std::string& payload) | ||
| { | ||
| assertHasCommand(command_str); |
There was a problem hiding this comment.
Why is this function not throwing not implemented error as the sendRequstString is?
| EXPECT_TRUE(dashboard_client_->commandStop()); | ||
| EXPECT_FALSE(dashboard_client_->commandRunning()); | ||
| EXPECT_TRUE(dashboard_client_->commandPowerOff()); | ||
| dashboard_client_->commandClosePopup(); // Necessary for CB3 test |
There was a problem hiding this comment.
This shouldn't be needed as everything is mocked.
|
|
||
| EXPECT_TRUE(dashboard_client_->commandLoadProgram("wait_program.urp")); | ||
| EXPECT_TRUE(dashboard_client_->commandPowerOff()); | ||
| dashboard_client_->commandClosePopup(); // Necessary for CB3 test |
There was a problem hiding this comment.
Shouldn't be needed as everything is mocked
Co-authored-by: Mads Holm Peters <79145214+urmahp@users.noreply.github.com>
2 participants
This adds the new Dashbaord Server called Robot API on PolyScope X 10.11.0 to the client library.
It is implemented as a drop-in replacement for the CB3 adn PolyScope 5 dashboard clients.
Note that this doesn't cover all features of the "old" Dashboard Server.