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

AP_Notify: Eliminated need for Solo specific tones, added power off tone to px4 tones #6218

Closed
wants to merge 1 commit into from

Conversation

Pedals2Paddles
Copy link
Contributor

Evaluated solo specific tones file vs standard px4 tones files. The only thing the Solo had that standard ArduPilot does not have is the GPS unplugged tone and the power off tone. This PR is needed to get Solo onto Master. Please backport into 3.5 if approved.

  • GPS unplugged tone abandoned. Determined to be unnecessary.
  • Power off tone merged into standard px4 tones file. Smart battery signalling a power off will make use of this tone. Has application for any smart battery equipped vehicle, not just Solo.
  • Removed all references and dependencies to ToneAlarm_PX4_Solo.cpp and ToneAlarm_PX4_Solo.h
  • Deleted ToneAlarm_PX4_Solo.cpp and ToneAlarm_PX4_Solo.h since they're no longer needed.

Evaluated solo specific tones file vs standard px4 tones files. The only
thing the Solo had that standard ArduPilot does not have is the GPS
unplugged tone and the power off tone. This PR is needed to get Solo
onto Master. Please backport into 3.5 if approved.
* GPS unplugged tone abandoned. Determined to be unnecessary.
* Power off tone merged into standard px4 tones file. Smart battery
signalling a power off will make use of this tone. Has application for
any smart battery equipped vehicle, not just Solo.
* Removed all references and dependencies to ToneAlarm_PX4_Solo.cpp and
ToneAlarm_PX4_Solo.h
* Deleted ToneAlarm_PX4_Solo.cpp and ToneAlarm_PX4_Solo.h since they're
no longer needed.
@Pedals2Paddles
Copy link
Contributor Author

@khancyr I think this one worked :)

@Pedals2Paddles Pedals2Paddles added this to Pull Requests in Solo on Master May 9, 2017
@khancyr
Copy link
Contributor

khancyr commented May 9, 2017

Yep, seem good to go.

@OXINARF
Copy link
Member

OXINARF commented May 10, 2017

I haven't give it a detailed review yet, but a few points:

  • your description makes it look like sounds are the same and many are different. Initial idea was to make Solo sounds available with a parameter, but if this changed that's even better
  • there is also a AP_NOTIFY_PX4_TONE_QUIET_DISARMED in Solo tones that might be interesting for the general driver

@proficnc
Copy link
Contributor

@OXINARF
We had a chat, and decided that if it's going to be solo on Ardupilot, it should sound like Ardupilot :)

Less messing around..

@OXINARF
Copy link
Member

OXINARF commented May 10, 2017

@proficnc Great, love it!

if (!flags.powering_off) {
play_tone(AP_NOTIFY_PX4_TONE_QUIET_SHUTDOWN);
}
flags.powering_off = AP_Notify::flags.powering_off;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line will always be true. Might as well just set it to true for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean? The AP_Notify::flags.powering_off flag will be false unless the user pushes the power button fox 2 seconds, at which point it goes true. The flags internal to the tone file are just to keep it from repeating once it's fired once. Otherwise it be playing the tone at 50hz.

@Pedals2Paddles
Copy link
Contributor Author

@tridge @rmackay9 Can this be merged? I have other work critical to solo that requires this to be in first.

@Pedals2Paddles
Copy link
Contributor Author

Disregard. I included this in a larger PR #6340

@Pedals2Paddles Pedals2Paddles removed this from Pull Requests in Solo on Master May 31, 2017
@Pedals2Paddles Pedals2Paddles deleted the solotones branch July 16, 2017 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants