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 buzzer feedback on GPS toggle #5090

Merged
merged 5 commits into from
Oct 18, 2024
Merged

Conversation

Technologyman00
Copy link
Contributor

Addresses #5089. Screen-less devices don't have any feedback for a triple click of the button so it maybe difficult to ensure the GPS was disabled

Add playbeep to triple press GPS to provide feedback for screenless devices
fix some readability
@CLAassistant
Copy link

CLAassistant commented Oct 18, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@fifieldt
Copy link
Contributor

This is a great idea, thank you!

@Technologyman00
Copy link
Contributor Author

@fifieldt Thank you! After playing around with it, it probably should have some audio cue for on vs off, but I am not a musician.

@fifieldt
Copy link
Contributor

Maybe since the disable sequence is a triple push, the associated buzzer should be a triple sound?

@Technologyman00
Copy link
Contributor Author

Triple click enables and disables. I will play around and see

One beep for GPS enable and the same beep for disable made it impossible to know which is which so now it has distinct different sounds.
@b8b8
Copy link

b8b8 commented Oct 18, 2024

triple low sound after disabling, triple high sound after enabling?

@Technologyman00
Copy link
Contributor Author

The most recent commit sounds like this: https://youtu.be/VHLxfraEB0k?si=R2N3lvpfyCn2dF4E

@Technologyman00
Copy link
Contributor Author

Don't merge yet the noise is backwards according to what the app reports

@fifieldt
Copy link
Contributor

Thanks so much for providing the video - helps a lot as I haven't time to flash and test. I think the sound is good enough to ship - we can always change later if needed :)

@Technologyman00 - are you familiar with the trunk linting/formatting tool? We use it to keep out files formatted the same. It looks like it needs to be run on the files from this patch. Could you do that? If not , let us know and someone will help. Other than that, good to go.

@fifieldt
Copy link
Contributor

We should also make sure this works OK on devices that don't have a buzzer.

The sounds were associated with the wrong function
@Technologyman00
Copy link
Contributor Author

Ok I just tested it on my T114 V1 and it works. I don't have a different one to test.

@fifieldt
Copy link
Contributor

OK, looks like buzzer.h is included by main and not guarded by PIN_BUZZER so will have no impact on devices without buzzers

Copy link
Contributor

@fifieldt fifieldt left a comment

Choose a reason for hiding this comment

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

Good to go apart from trunk format.

@Technologyman00
Copy link
Contributor Author

Someone else can do it or tell me why its been stuck doing this for the past while
image

Copy link
Contributor

@fifieldt fifieldt left a comment

Choose a reason for hiding this comment

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

Let's not make trunk take the fun out of your first patch.

@fifieldt fifieldt merged commit 934be69 into meshtastic:master Oct 18, 2024
48 checks passed
panaceya added a commit to mesh-mk-ua/meshtastic-firmware that referenced this pull request Oct 18, 2024
* Coerce minimum telemetry interval of 30 minutes on defaults and make new default interval one hour (meshtastic#5086)

* Coerce minimum telemetry interval of 30 minutes on defaults and make new default interval one hour

* Smaller log messages

* Add buzzer feedback on GPS toggle (meshtastic#5090)

Triple Press on buttons toggles GPS enable/disable.

This enhancement plays a triple-beep so that users of devices with buzzers can get audible feedback about whether they have turned the GPS off or on. This is especially valuable for screenless devices such as the T1000E where it may not be immediately obvious the GPS has been disabled.

---------

Co-authored-by: Ben Meadors <[email protected]>
Co-authored-by: Technologyman00 <[email protected]>
@Nestpebble
Copy link
Contributor

@Technologyman00 re: #5090 (comment)

Trunk doesn't work on windows for some reason. There's workarounds, either making it work in Windows by placing the files manually or running it in WSL, but I found the quickest way for a casual update was gitpod. YMMV.

Love this idea and implementation, btw. I have lots of screenless GPS devices, and this is really cool.

panaceya added a commit to mesh-mk-ua/meshtastic-firmware that referenced this pull request Oct 18, 2024
* Coerce minimum telemetry interval of 30 minutes on defaults and make new default interval one hour (meshtastic#5086)

* Coerce minimum telemetry interval of 30 minutes on defaults and make new default interval one hour

* Smaller log messages

* Add buzzer feedback on GPS toggle (meshtastic#5090)

Triple Press on buttons toggles GPS enable/disable.

This enhancement plays a triple-beep so that users of devices with buzzers can get audible feedback about whether they have turned the GPS off or on. This is especially valuable for screenless devices such as the T1000E where it may not be immediately obvious the GPS has been disabled.

---------

Co-authored-by: Ben Meadors <[email protected]>
Co-authored-by: Technologyman00 <[email protected]>
caveman99 pushed a commit that referenced this pull request Nov 3, 2024
Triple Press on buttons toggles GPS enable/disable.

This enhancement plays a triple-beep so that users of devices with buzzers can get audible feedback about whether they have turned the GPS off or on. This is especially valuable for screenless devices such as the T1000E where it may not be immediately obvious the GPS has been disabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants