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

MotoROS v1.7.0 #191

Merged
merged 6 commits into from
Jan 24, 2018
Merged

MotoROS v1.7.0 #191

merged 6 commits into from
Jan 24, 2018

Conversation

ted-miller
Copy link
Contributor

  • Implement velocity feedback for DX200 and YRC1000 controllers.
  • Remove unneeded validation check from RosSetupValidation.xLib.
  • Fix starting position check for SLUBT robots.

- Implement velocity feedback for DX200 and YRC1000 controllers.
- Remove unneeded validation check from RosSetupValidation.xLib.
- Fix starting position check for SLUBT robots.
@ted-miller
Copy link
Contributor Author

Eric Marcil will do additional testing this week.

- On systems with multiple control groups, only one reboot will be
required after activating the speed feedback registers.
- Fix calculation of speed feedback for linear axis.
@ted-miller
Copy link
Contributor Author

Updated based on testing by Marcil.

@@ -227,7 +227,7 @@ BOOL Ros_Controller_Init(Controller* controller)
if(grpNo < controller->numGroup)
{
// Determine if specific group exists and allocate memory for it
controller->ctrlGroups[grpNo] = Ros_CtrlGroup_Create(grpNo, controller->interpolPeriod);
controller->ctrlGroups[grpNo] = Ros_CtrlGroup_Create(grpNo, (grpNo==(controller->numGroup-1)), controller->interpolPeriod);
Copy link
Member

Choose a reason for hiding this comment

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

Could this line get a bit more of a comment?

It probably becomes clearer when looking up the declaration of Ros_CtrlGroup_Create(..), but what is the boolean there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New commit has additional comments.

long pulsePos[MAX_PULSE_AXES]);
UCHAR Ros_CtrlGroup_GetAxisConfig(CtrlGroup* ctrlGroup);
BOOL Ros_CtrlGroup_IsRobot(CtrlGroup* ctrlGroup);

Copy link
Member

Choose a reason for hiding this comment

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

These have moved to the header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, they were located in both the header and the source. I removed the duplication.

dblRegister *= ctrlGroup->pulseToRad.PtoR[i]; //pulse/sec
if (ctrlGroup->axisType.type[i] == AXIS_ROTATION)
{
dblRegister /= 10000.0; //deg/sec
Copy link
Member

@gavanderhoorn gavanderhoorn Jan 15, 2018

Choose a reason for hiding this comment

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

Suggestion: use 1e4 (and 1e5 below)?

Or: 1e-4 (and 1e-5) and multiply.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

- Add comments for clarity.
- Remove obsolete parameter from the setup-validation library (DX100
only).
@gavanderhoorn
Copy link
Member

Just tested this on our FS100. No regressions as far as I've tested.

I cannot test velocity reporting, nor the new SLUBT starting position check.

- Fix initialization of curPos and prevPulsePos when using B axis
compensation (for SLUBT robots).
@ted-miller
Copy link
Contributor Author

The latest SLUBT update has been tested on a live controller in simulation mode. It has not been tested on physical arm. (But I am very confident that it will be ok.)

@gavanderhoorn
Copy link
Member

@ted-miller wrote:

tested on a live controller in simulation mode.

you'll have to teach me how to do that one day.

Copy link
Member

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

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

Changes have been tested as best we can, let's iterate on improvements in new PRs.

@gavanderhoorn
Copy link
Member

I'll wait on Travis to complete and then merge.

💯 @ted-miller: this is a really nice implementation of velocity feedback I believe.

@gavanderhoorn
Copy link
Member

This would be something for a follow-up PR, but: would there be some way to implement a UI / settings panel of some sorts using MotoPlus? You mentioned in a private email that the implementation of the velocity feedback is quite 'resource intensive' (M-registers) and that it could complicate integration of MotoROS with other jobs on the controller.

Would it be an idea to give users a choice, so that they can disable the feedback if they're not interested in it and avoid the resource usage (which would be unneeded in that case)?

@ted-miller
Copy link
Contributor Author

ted-miller commented Jan 23, 2018

TM: tested on a live controller in simulation mode.

GH: you'll have to teach me how to do that one day.

Unfortunately, the Yaskawa password is required to reconfigure your controller for a different arm.

@ted-miller
Copy link
Contributor Author

would there be some way to implement a UI / settings panel of some sorts

This is definitely possible. There is an SDK available to develop a custom GUI application on the programming pendant. The development kit is a paid option, but the compiled binary and installer could be posted online.

Let me know if you have some ideas for things that would be good to have configurable, beyond just the velocity feedback. Maybe enable/disable torque feedback? Or display a diagnostic monitor screen? Perhaps display all the controller information that is calculated at startup (max speeds, pulse->radian, etc)?

@gavanderhoorn
Copy link
Member

@ted-miller: I've opened #193 to discuss the UI.

@gavanderhoorn
Copy link
Member

TM: tested on a live controller in simulation mode.

GH: you'll have to teach me how to do that one day.

Unfortunately, the Yaskawa password is required to reconfigure your controller for a different arm.

so ... ? ;)

@gavanderhoorn gavanderhoorn merged commit 4b9fd56 into ros-industrial:kinetic-devel Jan 24, 2018
@gavanderhoorn
Copy link
Member

This is a very nice addition to MotoROS @ted-miller. Thanks for this.

Now all I need to do is find a DX200/YRC1000 somewhere ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants