ecocurious2 / MultiGeiger

Geigerzähler mit ESP32 und empfindlichem Si22g-Zählrohr (Gamma)
https://multigeiger.readthedocs.io/
GNU General Public License v3.0
71 stars 30 forks source link

Continuous tone/beep in release 1.15.0 #398

Closed M-a-r-i-a-n-n-e closed 3 years ago

M-a-r-i-a-n-n-e commented 3 years ago

As soon as I set the speaker tick and the start sound to 0 in the web config, the Multigeiger cries at full volume with its piezo speaker after the following restart.

@ThomasWaldmann fyi, unfortunately it occurs again and it also occurs when using your binary.

ThomasWaldmann commented 3 years ago

Can't reproduce. Did 10x reset and 3x power-off/on, but i don't get a continuous tone.

ThomasWaldmann commented 3 years ago

https://github.com/ecocurious2/MultiGeiger/issues/352#issuecomment-803656139

so, close this?

ThomasWaldmann commented 3 years ago

Maybe related to #351 ?

M-a-r-i-a-n-n-e commented 3 years ago

Maybe related to #351 ?

I don't think it is related, because I started several times with a fully erased memory, did the flash and afterwards set both parameters to 0 again (usually after setting the Web- and the WiFi settings).

However, the continuous tone also was the result when I did the update (without erasing memory) from the previous versions (V1.13 for one multigeiger and V1.14 for the second one), as I described in #352 (comment). But I doubt it has to do with #351 as the config did not change so everything should have been initialized anyway.

M-a-r-i-a-n-n-e commented 3 years ago

As the sound just stops as soon as I turn at least the start sound on again, we should look for an initialization problem.

Unfortunately a soft reset does not lead to the issue.

M-a-r-i-a-n-n-e commented 3 years ago

For me, the following workaround in speaker.cpp (line 206, i.e. after the if (playSound) block) did the trick (and saved my ears! 🙂👂 ). You hear nothing and everything is properly initialized.

  } else {
    // Workaround to prevent https://github.com/ecocurious2/MultiGeiger/issues/398
    static int seq[][4] = {
      TONE(0, 0, 0, 0)
    };
    play((int *)seq);
  }

I do not create a pull request, because I hope someone will find a solution, not just a workaround.

t-pi commented 3 years ago

Ah, that reminds me, @M-a-r-i-a-n-n-e, what buzzer did you use, the one provided by @WwwBoehriDe?

I once also built a Multigeiger with some buzzer I had lying around - and this one had inverted control, i.e. normally on. Your case sounds similar - and I did indeed modify my code in a similar way for this specific device...

M-a-r-i-a-n-n-e commented 3 years ago

Ah, that reminds me, @M-a-r-i-a-n-n-e, what buzzer did you use, the one provided by @WwwBoehriDe?

Yes, the small one.

I once also built a Multigeiger with some buzzer I had lying around - and this one had inverted control, i.e. normally on. Your case sounds similar - and I did indeed modify my code in a similar way for this specific device...

I don't think so, because I also tried to set MCPWM_DUTY_MODE_1 and it didn't solve this issue.

ThomasWaldmann commented 3 years ago

Just as a general note:

The hard- and software requires the use of a passive piezo buzzer (== 2 wires directly contacting the conductive sides of the quartz). Buzzers with own built-in frequency generator or other active components of course do not work as intended.

A speciality of the multigeiger hard- and software is that the piezo wires get phase-inverted signals (1/0 ... 0/1 ... 1/0 ...).

ThomasWaldmann commented 3 years ago

@M-a-r-i-a-n-n-e good catch! that was not just a "workaround", but the fix. :-)

See PR #403. Can you test that code? As you always(?) had the issue and I didn't, guess it is better if you could test it.

ThomasWaldmann commented 3 years ago

@rexfue told me: "With Serial-USB logging connected / logger open, the beep is not heard."

Guess that explains why I never had the issue, because I usually have the logger open...

M-a-r-i-a-n-n-e commented 3 years ago

See PR #403. Can you test that code? As you always(?) had the issue and I didn't, guess it is better if you could test it.

Works for me. 👍 (please link the pull request #403 as I'm not allowed to)

As soon as #401 is fixed, too, we might think about a new minor release, because both issues are rather severe.