search

ClemensElflein / OpenMower

Let's upgrade cheap off-the-shelf robotic mowers to modern, smart RTK GPS based lawn mowing robots!
Other
4.64k stars 274 forks source link

Improve Sound #80

Open Apehaenger opened 10 months ago

Apehaenger commented 10 months ago

Think this is how it can be done.

This PR mainly:

SD-Card Structure Problem (old/new sound implementation):

I assumed that >90% of the installations in the wild, will have the original DFRobot DFPlayer with the old SD-Card-Structure installed. Because DFRobot's DFPlayer has the quirk to auto-play the first sound in SD-Cards root, after a reset(), I detect an old SD-Card-Structure in that way, that if there's an auto-played sound with a specific length (shorter than the one of the new SD-Card-Structure) then it's identified as "old SD-Card-Structure". For sure, this works only for those who have an DFRobot DFPlayer.

This (shall) result in the following behavior (tested with a couple of DFPlayer and SD-Cards):

DFPlayer SD-Card-Structure Plays
DFRobot old "Hi", krk, "Hi, I'm Steve ...". No further sound output
DFRobot new "Ping" (3-5 times), "Hi I'm Steve ...". No further sound output

Overcurrent problem on 3.3V (Pico) line

All current OM installation (with sound) already play "Hi", krk, "Hi, I'm Steve ..." and we haven't heard of died Pico (VREG) till now. So this sound PR also stop (by default) after "Hi, I'm Steve ...", except it got compiled with DFPIS5V (already added to CI firmware build) For sure this doubles our firmware binaries :-/ But does it matter?

Warning Of course, everybody is free to install a DFPIS5V firmware without changing his DFPlayer to 5V, but we should expect that he has read README-Sound, DFPIS5V.md before, and knows about the risk

firmware.zip

ClemensElflein commented 5 months ago

Thank you for the PR, that looks good to me. I'll install and test this before merging.

Two questions:

Apehaenger commented 5 months ago

... Two questions:

Instead of having the additional firmware versions to enable sound for 3.3V people, should we maybe just enable it through the openmower config on the linux side? I.e. the firmware always supports sound, but does not play any sound until ROS is booted up and tells the LowLevel to enable sound. If we want to play sound before ROS is booted, we can store the last config in Flash (with the default being OFF, so after one successful ROS boot, the sound will then work even before ROS is started). WDYT?

Good idea! But:

  1. I need some hint (pointing finger to some similar code within ROS) as I'm still unfamiliar with ROS
  2. Need some weeks to implement, because I'm somehow busy ATM

Nice collection of sound effects! I'm wondering if we can host them legally here though, since there are some songs in it?

Yes, already thought about it while collecting them. Most (or all) of the synthesized sounds are mainly extracted out of sounds from https://pixabay.com whose license looked okay to me like: https://pixabay.com/sound-effects/success-02-68338/ , https://pixabay.com/sound-effects/search/evil-robot-or-spaceship/

or from , https://soundcloud.com where it's more difficult to identify the license because each Author may have it's own copyright restrictions. Take i.e. https://soundcloud.com/agentjchicago/despicable-me-ringtone-beedo?in=user50805592/sets/kids where I expected some copyright notice, but haven't found them.

But for sure, regarding the songs like "Highway to Hell" I didn't made me the work to look for a "free covered song". Instead of I thought that we only "spoil?" the song and never play it complete. Because you fear about it, I just did a quick search and found the "15 seconds rule" (https://ronaldkah.de/15-sekunden-regel-musik/) which mainly says:

Die folgenden Nutzungen von Werken Dritter gelten als geringfügig im Sinne des § 9 Absatz 2 Satz 1 Nummer 3, sofern sie nicht zu kommerziellen Zwecken oder nur zur Erzielung unerheblicher Einnahmen dienen: 

1.        Nutzungen bis zu 15 Sekunden je eines Filmwerkes oder Laufbildes,
2.        Nutzungen bis zu 15 Sekunden je einer Tonspur,
3.        Nutzungen bis zu 160 Zeichen je eines Textes und
4.        Nutzungen bis zu 125 Kilobyte je eines Lichtbildwerkes, Lichtbildes oder einer Grafik.

Quelle: Bundesamt für Justiz, https://www.gesetze-im-internet.de/urhdag/__10.html

So, indeed, some of the songs are approx. 30 sec. long. If we want to be on the save side, we've to shorten them.

But this would only fulfill the German law! :-/

ClemensElflein commented 5 months ago

No worries, I can also do the ROS side implementation of this, just wanted to hear your input on this one. Thank you for the write-up regarding the sounds. I think we should remove the potentially problematic sounds from the repo not that this is the reason it gets taken down some day.

Apehaenger commented 5 months ago

Quite cool for the ROS part. I'm pretty sure you already have a detailed view in your brain, but pls. let me drop in my thoughts: I think we need some kind of "HighLevel-Config" struct packet with more than only "enable sound". Probably also interests are in:

  1. (sound) language
  2. (sound) volume (even if I'm unsure ATM if it should remain by CoverUI controlled or better should go into WebGUI)
  3. Inverse Hall-x handling for SA/SC types (due to their inverted logic)
  4. Rain sensor sensitivity (because Classic and SA (as well as probably SC) type mower have different rain sensors

Sounds: Let me offer to check if I find "free covers" of them. If not, I'll drop them.

Do you prefer to stay within this PR or shall I build a new cleaner one, once all is implemented?

ClemensElflein commented 5 months ago

Yes exactly, I thought about a configuration packet where we can set key/value pairs. LL board shall store the configuration and only allow to exit emergency once it has some valid configuration.

I don't really have a preference, if your implementation builds on this one, we can continue this thread.

Apehaenger commented 5 months ago

Did a quick look to open_mower_ros for this sound + #84 PR integration. Doesn't look very complicated.

  1. I could add a ll_config struct with i.e. 'dfp_is_5v' as well as a 'language' member (ll_datatypes.h & mower_comms.cpp) and let him fire the packet i.e. once a second
  2. Mod openmower FW in that way that
    1. a received ll_config member get stored in flash
    2. Emergency only get released once there's a valid config stored

But I've some questions/doubts:

  1. Once LL FW has a valid config stored, it wouldn't wait for a ll_config packet again and play sound immediately (with the option that an ll_config get received which disabled sound again via dfp_is_5v=false
  2. Wouldn't it be better if LL FW request HL for an ll_config packet instead of cyclic send by HL?
Apehaenger commented 5 months ago

Yes exactly, I thought about a configuration packet where we can set key/value pairs. LL board shall store the configuration and only allow to exit emergency once it has some valid configuration.

Just checked how to implement https://github.com/ClemensElflein/open_mower_ros/pull/79 in LowLevel code. Think we shouldn't lock in emergency due to our latest discussion about packet (backward) compatibility. In addition my current https://github.com/ClemensElflein/open_mower_ros/pull/79 implementation will be somehow fragile when i.e. flashing pico with a running open_mower_ros. In special if someone change to DFPIS5V=false and flashes Pico afterwards. He wouldn't get a config packet but would use the old flash-stored configuration probably with a previous DFPIS5V=true!

Think we either need a cyclic send (i.e. 1-10s) config packet from HL side, or a separate "getConfig" packet from LL which request HL to send the config packet. I personally would prefer the cyclic send config packet, because it would give us easier possibility to change settings on the fly (some when)

Apehaenger commented 4 months ago

Merged all together now. Did a couple of tests:

Look all fine and it's working on my desktop. If weather is good on Sunday, I'll do some final practice tests.

Please take into account that the following PR is also needed: https://github.com/ClemensElflein/open_mower_ros/pull/80

If interested in a quick test, binaries are already available in my fork: https://github.com/Apehaenger/OpenMower/releases/tag/latest

Apehaenger commented 4 months ago

Just started a real practice test on my SA type mower. All looks working as expected. Don't see any issues.

ClemensElflein commented 4 months ago

before merging we should mention the "the player just plays everything"-issue somewhere (missing resistors in 0.13 kit); also if there is a guide somewhere we should mention those resistors.

Apehaenger commented 6 days ago

Finished all new sound capabilities (by the use of the HL-State-SubModes), changes and ideas. Just succeed test on second mower. Work as expected.