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

Favorite-able WAV files #97

Merged
merged 3 commits into from
Apr 17, 2024
Merged

Favorite-able WAV files #97

merged 3 commits into from
Apr 17, 2024

Conversation

zacharyweiss
Copy link
Contributor

@zacharyweiss zacharyweiss commented Apr 17, 2024

What's new

Primarily just wanted this feature for the sake of favorite-ing the cart lock/unlock WAVs.

  • Allows archive to recognize .wav files as openable by wav_player; as such enables "favorite" menu item
  • Makes basic edits to wav_player to allow "direct launching" from a file, rather than requiring a file browser dialog to select the file after starting the FAP.

The latter is messy / very quickly done as proof-of-concept. Doesn't yet impl the "favorite timeout" feature (have yet to use this myself so not actually sure what it does). Aside from that, anything missing that needs implementing to finalize this as a feature?

Given the required changes across both this repo and the submoduled apps repo, what's the best form for PR here? Shall I open a separate PR on the apps repo and reference the two?


For the reviewer

  • I've uploaded the firmware with this patch to a device and verified its functionality
  • I've confirmed the bug to be fixed / feature to be stable

Namely just for the sake of favorite-ing the cart lock/unlock. Requires changes to WAV player ext fap, as to be PR'd from the Momentum-Apps repo.
@Willy-JL
Copy link
Member

Given the required changes across both this repo and the submoduled apps repo, what's the best form for PR here? Shall I open a separate PR on the apps repo and reference the two?

that sounds like the best course of action, yes

Copy link
Member

@Willy-JL Willy-JL left a comment

Choose a reason for hiding this comment

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

LGTM, but will wait to merge as i'm guessing by merging the PR into apps repo the commit ID will change for the submodule

applications/external Show resolved Hide resolved
@zacharyweiss
Copy link
Contributor Author

Great! PR'd the apps repo; once that's merged will push another commit here if needed for handling the commit ID bookkeeping

@Willy-JL Willy-JL added the feature New feature or request label Apr 17, 2024
Copy link
Member

@Willy-JL Willy-JL left a comment

Choose a reason for hiding this comment

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

thanks!

@Willy-JL
Copy link
Member

about the favorite timeout, that should be mostly irrelevant. the idea for it is to quit favorite files after a configured amount of time, makes sense for things like subghz files and nfc and rfid, not so much for wav files

@Willy-JL Willy-JL merged commit 0d794d0 into Next-Flip:dev Apr 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants