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

Support for DX200 controllers #49

Merged
merged 14 commits into from
Aug 14, 2023
Merged

Support for DX200 controllers #49

merged 14 commits into from
Aug 14, 2023

Conversation

gavanderhoorn
Copy link
Collaborator

@gavanderhoorn gavanderhoorn commented Jun 2, 2023

Edit 2: current status:

Remaining TODO:

  • clean up commit history

Edit: current status:

  • Humble and Foxy builds appear to work on DX200, but
  • IP addresses with last octet >= 128 will result in a Must Enable ETHERNET function (ie: Alarm 8011) to be raised. Anything with last octet < 128 is expected to work (as a work-around): Support for DX200 controllers #49 (comment): this should no longer be a problem.

Available test builds:


This is for #18.

A draft currently, but first steps towards full DX200 support.

@gavanderhoorn gavanderhoorn added this to the 0.1.1 milestone Jun 2, 2023
@gavanderhoorn gavanderhoorn linked an issue Jun 2, 2023 that may be closed by this pull request
9 tasks
@gavanderhoorn

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as off-topic.

@ted-miller

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

src/CtrlGroup.c Outdated Show resolved Hide resolved
@gavanderhoorn gavanderhoorn mentioned this pull request Jun 10, 2023
9 tasks
@gavanderhoorn

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@acbuynak

This comment was marked as resolved.

@acbuynak

This comment was marked as resolved.

@gavanderhoorn

This comment was marked as resolved.

@acbuynak

This comment was marked as resolved.

@ted-miller

This comment was marked as resolved.

@gavanderhoorn gavanderhoorn force-pushed the dx200_support branch 2 times, most recently from be92a74 to 2315a0f Compare August 10, 2023 06:29
README.md Outdated Show resolved Hide resolved
@gavanderhoorn
Copy link
Collaborator Author

While I can't test it, I agree with @ted-miller this is in quite an OK shape.

I'll spend some time fixing up the commit history and then we should be good to merge.

gavanderhoorn and others added 10 commits August 12, 2023 17:51
Note: this just prevents GCC from immediately exiting, more changes are needed to make MotoROS2 compatible with DX200 platforms.
DX200 doesn't have this function, so we have to provide a custom implementation.

For YRC just forward the call.
A platform-specific mpCtrlGrpNo2GrpId(..) wrapper.
@gavanderhoorn
Copy link
Collaborator Author

Candidate commit history: main...gavanderhoorn:motoros2:dx200_support-squashed.

@ted-miller: I believe those commits succinctly show which changes were/are needed to get the YRC version working on DX2 without including all the fixups and experiments we needed to get to this point.

If you agree with the proposed set of commits, I'll force push it here (ie: overwrite the current dx200_support branch).

ted-miller and others added 4 commits August 14, 2023 08:07
Co-authored-by: G.A. vd. Hoorn <[email protected]>
At least in code touched by adding DX200 support.

Always check older platforms first.

Always explicitly check supported platforms (avoid inverse checks).

Always use 'if defined' instead of 'if' (the latter is deprecated).
@ted-miller
Copy link
Collaborator

Your branch looks good. Go ahead and push/merge it.

@gavanderhoorn
Copy link
Collaborator Author

Going to need a final review (again).

We could then merge this, deal with some of the other outstanding PRs and decide whether that'd be sufficient for 0.1.1.

We could also decide to go for 0.2.0, seeing as DX200 support is a bit of a milestone (also: 0.2.0 matches nicely with DX200, but that's minor).

@ted-miller
Copy link
Collaborator

We could then merge this, deal with some of the other outstanding PRs and decide whether that'd be sufficient for 0.1.1

See #120

@ted-miller ted-miller merged commit 70c0d83 into main Aug 14, 2023
@ted-miller ted-miller deleted the dx200_support branch August 14, 2023 18:01
@gavanderhoorn
Copy link
Collaborator Author

Thanks for the work @ted-miller 👍

Thanks for testing early builds and putting up with our mistakes @acbuynak and @smith-doug 👍

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.

Parameter Extraction Library should check S2C1117 = 1 on DX200 Add support for DX200
4 participants