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

Add support for using Safety switch pins on IOMCU to control ProfiLEDs via bit banging #27732

Merged
merged 6 commits into from
Nov 20, 2024

Conversation

bugobliterator
Copy link
Member

@bugobliterator bugobliterator commented Aug 2, 2024

  • CPU time cost is minimal, only upto 7us per call is consumed.
  • Flash and Data Cost is as follows:

Before:

BUILD SUMMARY
Build directory: /home/sidbh/Programming/ardupilot-chibios/build/iomcu-f103-dshot
Target                   Text (B)  Data (B)  BSS (B)  Total Flash Used (B)  Free Flash (B)  External Flash Used (B)
-------------------------------------------------------------------------------------------------------------------
bin/iofirmware_lowpolh      52404      1196    19300                 53600            7836  Not Applicable
bin/iofirmware_highpolh     52404      1196    19300                 53600            7836  Not Applicable

After

BUILD SUMMARY
Build directory: /home/sidbh/Programming/ardupilot-chibios/build/iomcu-f103-dshot
Target                   Text (B)  Data (B)  BSS (B)  Total Flash Used (B)  Free Flash (B)  External Flash Used (B)

-------------------------------------------------------------------------------------------------------------------
bin/iofirmware_lowpolh      52160      1196    19296                 53356            8084  Not Applicable
bin/iofirmware_highpolh     52160      1196    19296                 53356            8084  Not Applicable

@bugobliterator bugobliterator force-pushed the pr-iomcu-profiled branch 2 times, most recently from 60c98c3 to a350c11 Compare August 2, 2024 05:32
@andyp1per andyp1per changed the title Add support for using Safety switch pins to control ProfiLEDs via bit banging Add support for using Safety switch pins on IOMCU to control ProfiLEDs via bit banging Aug 5, 2024
Copy link
Collaborator

@andyp1per andyp1per left a comment

Choose a reason for hiding this comment

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

I think this is ok. bitbanging in the main iomcu loop makes me a bit nervous because we are so short on time. dshot on F100 would probably need some careful testing.

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

we can't lose the BRD_SAFETY_DFLT feature, needed by many users

@@ -157,7 +157,7 @@ const AP_Param::GroupInfo AP_BoardConfig::var_info[] = {
// @Param: SAFETY_DEFLT
// @DisplayName: Sets default state of the safety switch
// @Description: This controls the default state of the safety switch at startup. When set to 1 the safety switch will start in the safe state (flashing) at boot. When set to zero the safety switch will start in the unsafe state (solid) at startup. Note that if a safety switch is fitted the user can still control the safety state after startup using the switch. The safety state can also be controlled in software using a MAVLink message.
// @Values: 0:Disabled,1:Enabled
// @Values: 0:Disabled,1:Enabled,3:Use Safety pin to control ProfiLED
Copy link
Contributor

Choose a reason for hiding this comment

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

we still need BRD_SAFETY_DEFLT, most vehicles these days have safety state controlled via mavlink in software, not via a button

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed by creating a separate parameter that enabled IO Profiled control. Also tested if Safety switch control continue to operate as before through mission planner.

@tridge tridge removed the DevCallEU label Aug 21, 2024
@bugobliterator bugobliterator force-pushed the pr-iomcu-profiled branch 2 times, most recently from 6f1b301 to a7e061f Compare August 28, 2024 06:23
@@ -370,6 +370,16 @@ const AP_Param::GroupInfo AP_BoardConfig::var_info[] = {
// @User: Advanced
AP_GROUPINFO("IO_DSHOT", 28, AP_BoardConfig, state.io_dshot, 0),
#endif

#if HAL_WITH_IO_MCU
// @Param: IO_PROFILED
Copy link
Contributor

Choose a reason for hiding this comment

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

BRD_OPTIONS makes more sense I think

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Aug 29, 2024
@bugobliterator bugobliterator force-pushed the pr-iomcu-profiled branch 2 times, most recently from 72cf9b0 to 6e5d578 Compare August 30, 2024 01:55
@bugobliterator bugobliterator requested a review from tridge August 30, 2024 02:28
Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

we've discussed, and think this should be a iomcu_CubePilot build, and not make any fw change on other variants
also needs some dshot testing by @andyp1per

@tridge tridge removed the DevCallEU label Sep 4, 2024
@bugobliterator bugobliterator force-pushed the pr-iomcu-profiled branch 2 times, most recently from c521c2c to b0a0813 Compare September 17, 2024 05:44
@bugobliterator bugobliterator force-pushed the pr-iomcu-profiled branch 2 times, most recently from b564035 to 5e70111 Compare September 18, 2024 04:18
@tridge
Copy link
Contributor

tridge commented Sep 25, 2024

for the ccache failure, I think we need to change it to be two other H7 boards we test, maybe Pixhawk6X and ZeroOneX6 ?

@tridge
Copy link
Contributor

tridge commented Sep 25, 2024

hmm, the ccache test (see test_ccache.yml) uses Durandal-bdshot and Pixhawk6X, which should be unaffected by this PR, which implies this PR is changing something it shouldn't

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.

Pixhawk6X is also diverging from Durandal somehow, causing ccache to test fail. That needs to be explained as this shouldn't really affect either board.

libraries/AP_Notify/AP_Notify.cpp Show resolved Hide resolved
@tridge tridge removed the DevCallEU label Oct 2, 2024
@peterbarker
Copy link
Contributor

Ping @bugobliterator - those plane trips have to be good for something? :-)

@bugobliterator bugobliterator force-pushed the pr-iomcu-profiled branch 9 times, most recently from 1dc3e88 to f84402f Compare October 15, 2024 09:36
Copy link
Collaborator

@andyp1per andyp1per left a comment

Choose a reason for hiding this comment

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

Testing seemed fine

@peterbarker
Copy link
Contributor

Board                    AP_Periph  blimp  bootloader  copter  heli  iofirmware  plane  rover  sub
CubeBlack                           776    *           776     776               776    776    776
CubeOrange-periph-heavy  408                                                                   
CubeOrangePlus                      784    *           784     776               784    776    776
CubeRedPrimary                      *      *           *       *                 *      *      *
Durandal                            *      *           *       *                 *      *      *
Hitec-Airspeed           *                                                                     
KakuteH7-bdshot                     *      *           *       *                 *      *      *
MatekF405                           *      *           *       *                 *      *      *
Pixhawk1-1M-bdshot                  *                  *       *                 *      *      *
f103-QiotekPeriph        *                                                                     
f303-Universal           *                                                                     
iomcu                                                                *                         
revo-mini                           *      *           *       *                 *      *      *
skyviper-journey                                       *                                       
skyviper-v2450                                         *                                       

@tridge
Copy link
Contributor

tridge commented Nov 20, 2024

i'd say that using the safety pin like this offends my sensibilities a bit, but holding my nose I will merge

@tridge tridge merged commit 55d8267 into ArduPilot:master Nov 20, 2024
98 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DevCallEU WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants