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

Roboclaw docs #2990

Merged
merged 12 commits into from
Jan 8, 2024
Merged

Roboclaw docs #2990

merged 12 commits into from
Jan 8, 2024

Conversation

PerFrivik
Copy link
Contributor

@PerFrivik PerFrivik commented Dec 15, 2023

Docs for the roboclaw motor controller driver that I improved.

@PerFrivik PerFrivik requested a review from MaEtUgR December 15, 2023 13:20
@hamishwillee
Copy link
Collaborator

@PerFrivik Can you ETA when you might be able to look at the fixes requested in #2990 (comment) ?

Specifically images the PR adds that aren't used, and making the changes in the /en rather than /de path.

I can then review this.

@PerFrivik
Copy link
Contributor Author

Sorry for the late reply, Ill try to finish it tomorrow (21.12).

@hamishwillee
Copy link
Collaborator

Thanks very much. Appreciate you have competing priorities.

As part of this could you add a release note into the en/releases/main.md topic. You can see the pattern here: https://docs.px4.io/main/en/releases/main.html
This is to avoid all the crap we normally have in releases where no one remembers what they have done and why :-)

@PerFrivik
Copy link
Contributor Author

@hamishwillee would this driver be under the hardware support section regarding the release notes?

@hamishwillee
Copy link
Collaborator

@hamishwillee would this driver be under the hardware support section regarding the release notes?

Yes. But it isn't so important where it goes as that it is there (i.e. we might move things later)

Comment on lines 56 to 57
- If using several RoboClaw motor controllers, each can be assigned a unique address on the bus, with the default address being 128.
To set a different address, specify it in the [RBCLW_ADDRESS](../advanced_config/parameter_reference.md#RBCLW_ADDRESS) parameter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This confuses me. If I have several motor controllers, and each must have its own address, how do I set them when I only have one parameter - RBCLW_ADDRESS?

FYI only, note that I do lots of cross linking, because it makes it easy to spot when people change the underlying implementations for parameters etc (since these often disappear).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaEtUgR This is actually a good question. Perfect that we ordered a second roboclaw, I'll test this next week.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So this looks good to me once this issue is resolved.

Copy link
Member

Choose a reason for hiding this comment

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

@hamishwillee You are asking exactly the right questions. To clarify: You need the correct Roboclaw address in the transaction header even if you only have one ESC. It needs to be the same that you configured with the Roboclaw configuration tool before over USB directly. We currently only have one ESC in our vehicle. The reason I would not spend too much time investigating yet is that AFAIK we are so far the only Roboclaw users on PX4. The driver improvements and docs should facilitate other users to get using it and the Aion R1/R6.

Using multiple ESCs on one vehicle was never tested and we should make that clear. We won't invest time into doing that until we need the functionality e.g. to make a Mecanum wheel Rover where all wheels need a separate controller. We'd have the hardware parts for that but it's currently not the focus/priority.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @MaEtUgR (and @PerFrivik ). So:

  1. Then we don't support multiple RoboClaw MCs, because there is only one param for setting the header sent by PX4. I have made this clear. Also noted that you should not have to change this, but if you do, then you must make the param match.
  2. I have added the information about Aion R6 to the intro too. I was wondering why we were using a discontinued frame. Helpful to be able to to day "this is still very useful".

I will merge, but if you have any issues with this change, please add a post PR to fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @hamishwillee ! I appreciate the help on getting this in.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries. It's pretty cool! Rover is much under loved.

@hamishwillee
Copy link
Collaborator

@PerFrivik Any chance of you looking at the questions here? (I assume you're on holiday, in which case, this is a reminder!)

@PerFrivik
Copy link
Contributor Author

@PerFrivik Any chance of you looking at the questions here? (I assume you're on holiday, in which case, this is a reminder!)

Thanks for the reminder Hamish! I am back now, will try to have this answered by this friday Jan 5.

en/releases/main.md Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jan 8, 2024

No flaws found

@hamishwillee hamishwillee merged commit c239468 into main Jan 8, 2024
1 of 2 checks passed
@hamishwillee hamishwillee deleted the roboclaw-docs branch January 8, 2024 21:57
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.

3 participants