Skip to content

AR_Motors: Make power limiting eaiser for end-users#32788

Open
stephendade wants to merge 2 commits intoArduPilot:masterfrom
stephendade:tclimit
Open

AR_Motors: Make power limiting eaiser for end-users#32788
stephendade wants to merge 2 commits intoArduPilot:masterfrom
stephendade:tclimit

Conversation

@stephendade
Copy link
Copy Markdown
Contributor

Summary

Based on the conversation at https://discuss.ardupilot.org/t/rover-4-7-0-beta2-available-for-beta-testing/143065/12, I've made a few change to the Rover power limiting feature to make it easier for users:

This includes:
-Notifying the user (once per 5 seconds) if power limiting is active
-Reducing the default MOT_BAT_WATT_TC from 5 to 2, to reduce lag in the low-pass filter.

Tested in SITL and a real rover.

Classification & Testing (check all that apply and add your own)

  • Checked by a human programmer
  • Non-functional change
  • No-binary change
  • Infrastructure change (e.g. unit tests, helper scripts)
  • Automated test(s) verify changes (e.g. unit test, autotest)
  • Tested manually, description below (e.g. SITL)
  • Tested on hardware
  • Logs attached
  • Logs available on request

Comment thread libraries/AR_Motors/AP_MotorsUGV.cpp Outdated
// warn the user every 5 seconds while limiting is active
if (_throttle_limit < _throttle_max) {
const uint32_t now_ms = AP_HAL::millis();
if (now_ms - _last_power_limit_warn_ms >= 5000U) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: shouldn't we create a variable to hold this time an have people to customize it as they need ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My guess is that if we think the messages will annoy users then we could add an option bit (MOT_OPTIONS?) to simply disable the text output. We probably wouldn't need to go as far as allowing them to customise how often it is sent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

agree on this. That is why I just ask for a code Constant. This won't expose it on gcs but from developper POV, it will make things easier

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm also not sure though that we should be alerting the user just because the watt limiter is limiting the throttle. If the system is working properly we normally don't inform the user. It's only when things go wrong that we inform the user. I guess we could interpret the motors going over their watt limit as "something going wrong" but I'm still unsure

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've added in a #define for the reporting interval

I guess we could interpret the motors going over their watt limit as "something going wrong" but I'm still unsure

I'd argue that it is. If the user's configured the watt limit, they must done it for a reason :)

The feedback's particularly useful as GCS's don't typically display the current power draw by the vehicle. So the user would otherwise have no feedback that the limiter's active - which may confuse them into thinking the throttle drop is due to some other issue.

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants