-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
libtunes improvements #9096
libtunes improvements #9096
Conversation
This prevents the expanction of the ERROR macro inside the TuneID enum
23b4e1a
to
7e6e065
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed & tested on fmu-v5, tunes still work.
While at it, can you please remove the duplicated startup tune in commander (line https://github.com/PX4/Firmware/blob/master/src/modules/commander/commander.cpp#L1532 and https://github.com/PX4/Firmware/blob/master/src/modules/commander/commander.cpp#L1538) - the startup tone is already in the ROMFS.
Addressing review comments in PR #9096
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was quick, thanks!
@bkueng Thanks for the review!
Good point, that also caused some problems for me in the past where the tune playback was chopped up because of the duplicate. I removed it in the new commit if you want to give it another test :) Edit: argh you were faster 😄 |
These are a few small baby steps towards improving libtunes:
src/lib/tunes/tune_definition.h
. This might facilitate custom implementations with different tune IDs for branding PX4.src/drivers/drv_tone_alarm.h
src/lib/tunes/tunes.cpp
src/systemcmds/tests/test_hrt.c
I don't currently have px4 hardware with buzzers at hand. Someone please test before merging!
@simonegu FYI