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.

Sign up for GitHub

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

Jump to bottom

AP_HAL_ChibiOS: add hwdef MFE_AirSpeed_CAN #28989

Merged
merged 2 commits into from
Jan 11, 2025
Merged

AP_HAL_ChibiOS: add hwdef MFE_AirSpeed_CAN #28989

merged 2 commits into from
Jan 11, 2025

Conversation

mikefenghao
Copy link
Contributor

This is the hwdef file for MFE_AirSpeed_CAN. Please help me merge. Thanks。

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Jan 3, 2025
@peterbarker peterbarker force-pushed the ardupilot-MFE_AirSpeed_CAN branch from aee3f6d to 1abf0da Compare January 9, 2025 21:53
@peterbarker
Copy link
Contributor

Is there some reason this hasn't been done by includeing eg. f103-periph/hwdef.inc (and the bootloader equivalent) and then adding only what needs to be added?

Copy-and-pasting entire hwdefs around like this increases maintenance load on the project and can lead to bugs. For example, several of the dfefines around serial use are changing (and have changed) in master since this copy/paste was done.

As an example of how this might be structured - AeroFox-Airspeed is doing what I describe.

@mikefenghao mikefenghao force-pushed the ardupilot-MFE_AirSpeed_CAN branch from 1abf0da to f3e6840 Compare January 10, 2025 06:55
@mikefenghao
Copy link
Contributor Author

Is there some reason this hasn't been done by includeing eg. f103-periph/hwdef.inc (and the bootloader equivalent) and then adding only what needs to be added?

Copy-and-pasting entire hwdefs around like this increases maintenance load on the project and can lead to bugs. For example, several of the dfefines around serial use are changing (and have changed) in master since this copy/paste was done.

As an example of how this might be structured - AeroFox-Airspeed is doing what I describe.

As you said, I have made changes.

@mikefenghao mikefenghao force-pushed the ardupilot-MFE_AirSpeed_CAN branch from f3e6840 to d3943f2 Compare January 10, 2025 07:06
peterbarker
peterbarker approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

LGTM

libraries/AP_HAL_ChibiOS/hwdef/MFE_AirSpeed_CAN/hwdef.dat Outdated
Comment on lines 19 to 20
# enable watchdog

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# enable watchdog

@mikefenghao mikefenghao force-pushed the ardupilot-MFE_AirSpeed_CAN branch 2 times, most recently from 9fe0335 to 69ed463 Compare January 10, 2025 08:12
@peterbarker peterbarker force-pushed the ardupilot-MFE_AirSpeed_CAN branch from 69ed463 to cd0bdb7 Compare January 10, 2025 08:41
@peterbarker peterbarker merged commit 9cb0d8e into ArduPilot:master Jan 11, 2025
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterbarker peterbarker peterbarker approved these changes

Assignees
No one assigned
Labels
MergeOnCIPass WikiNeeded needs wiki update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mikefenghao @peterbarker @Hwurzburg