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

Copter/Plane/Rover/Tracker: add PilotPi support #28247

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

heeplr
Copy link

@heeplr heeplr commented Sep 27, 2024

@rmackay9
Copy link
Contributor

Hi @heeplr,

Thanks for the contribution!

I'm not really qualified to review the changes but I can see that the single commit will need to be broken up into multiple commits with each affecting just a single directory. We've got some info on this in the developer wiki here.

@heeplr
Copy link
Author

heeplr commented Sep 28, 2024

I'm not really qualified to review the changes but I can see that the single commit will need to be broken up into multiple commits with each affecting just a single directory.

I saw this but I supposed it's meant for adding multiple features to different libraries. With this PR every commit msg would be "add pilotpi support" or "enable xy for pilotpi". Should I still do it although it doesn't really ease review?

@rmackay9
Copy link
Contributor

rmackay9 commented Sep 28, 2024

@heeplr,

It's fine for the commit messages to have similar titles but they should have a different prefix. So I think there should be commits with at least these prefixes:

  • "Tools: "
  • "AP_BattMonitor: "
  • "AP_HAL_Linux: "

@heeplr
Copy link
Author

heeplr commented Sep 28, 2024

@rmackay9 thank you for clarifying.

I ran git-subsystems-split and it worked like a charm.

@heeplr heeplr force-pushed the board-pilotpi branch 2 times, most recently from 855b284 to 1aeca1a Compare September 29, 2024 16:21
@heeplr
Copy link
Author

heeplr commented Sep 29, 2024

Please consider reviewing GPIO_PilotPi thoroughly. I copied GPIO_Navigator and adapted it to comply with the convention of AUX pins being >= 50 and MainOut pins being >= 101, but I don't really know what I'm doing ;)

@Hwurzburg
Copy link
Collaborator

@tridge @rmackay9 who is our Linux board reviewer.....I am not capable

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

Successfully merging this pull request may close these issues.

4 participants