M300 Beeps much higher pitch than they should be.

[KriLL3]

Upgraded from 1.0.2 to 1.1.0-RC2 and using the same g-code as before all of my M300 entries give pretty much the same really high pitched chirp instead of different tones denoted by the S variable, it's not a big issue but I liked having my printer make 2-4 note sounds based on it's state, (homing axis, heating bed, heating extruder, starting to print, done printing) and using a snippet of a song like a ringtone is pretty popular to put into the end g-code, but all those sounds are now indistinguishable high pitched squeaks.

I'm using RAMPS 1.4 with the discount full graphic smart controller

[Roxy-3D]

A long time ago, the M300 code looked like this:

    #if (LARGE_FLASH == true && ( BEEPER > 0 || defined(ULTRALCD) || defined(LCD_USE_I2C_BUZZER)))
    case 300: // M300
    {
      int beepS = code_seen('S') ? code_value() : 110;
      int beepP = code_seen('P') ? code_value() : 1000;
      if (beepS > 0)
      {
        #if BEEPER > 0
          tone(BEEPER, beepS);
          delay(beepP);
          noTone(BEEPER);
        #elif defined(ULTRALCD)
          lcd_buzz(beepS, beepP);
        #elif defined(LCD_USE_I2C_BUZZER)
          lcd_buzz(beepP, beepS);
        #endif
      }
      else
      {
        delay(beepP);
      }
    }
    break;
    #endif // M300

Now it looks like this:

  void buzz(long duration, uint16_t freq) {
    if (freq > 0) {
      #if ENABLED(LCD_USE_I2C_BUZZER)
        lcd_buzz(duration, freq);
      #elif PIN_EXISTS(BEEPER) // on-board buzzers have no further condition
        SET_OUTPUT(BEEPER_PIN);
        #if ENABLED(SPEAKER) // a speaker needs a AC ore a pulsed DC
          //tone(BEEPER_PIN, freq, duration); // needs a PWMable pin
          unsigned int delay = 1000000 / freq / 2;
          int i = duration * freq / 1000;
          while (i--) {
            WRITE(BEEPER_PIN, HIGH);
            delayMicroseconds(delay);
            WRITE(BEEPER_PIN, LOW);
            delayMicroseconds(delay);
           }
        #else // buzzer has its own resonator - needs a DC
          WRITE(BEEPER_PIN, HIGH);
          delay(duration);
          WRITE(BEEPER_PIN, LOW);
        #endif
      #else
        delay(duration);
      #endif
    }
    else {
      delay(duration);
    }

I guess we need to know the values of these #define's to figure out which code path it is going down in your code:

#define LCD_USE_I2C_BUZZER
#define BEEPER
#define SPEAKER

You can put a xxx(); inside one #if, #else or #elif section at a time to see if you get a compiler error. If not, that section of the code is not active and we don't care about it.

[KriLL3]

Hmm I can try that but I downloaded the 1.1.0-RC2 and only things I've changed was some esteps and similar calibration style changes in configuration.h and uncommented "#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER" don't have access to the machine with the modified configuration and arduino on it currently.

[Roxy-3D]

Well... Maybe somebody else can help you that knows more about RAMPS based systems. But without knowing which code paths are active it is very difficult to try to figure out where the problem is.

[KriLL3]

Ok I've tried it I get no error if I put it on the line after

#if ENABLED(SPEAKER) // a speaker needs a AC ore a pulsed DC (#12)

I get an error if I put it after

#else // buzzer has its own resonator - needs a DC (#22)

And no error if I put it after

#else (#27)

It's defiantly going down the DC route, which is weird, doesn't that only allow one tone? In 1.0.2 it could do a whole range just fine.

Did this on clean 1.1.0-RC2 with only change being removing the "//" on line 713 of configuration.h "#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER"

My best guess of what's going wrong here is that the configuration isn't right for that LCD controller anymore, marlin doesn't know what kind of speaker it has anymore but used to in 1.0.2.

[Roxy-3D]

Yes, it is possible the configuration isn't correct... But if you want to try to get back the old functionality, you can try a couple of things. You can replace buzz() with each possible combination and see if one of them does what you want

  void buzz(long duration, uint16_t freq) {
    if (freq > 0) 
        lcd_buzz(duration, freq);
  }

If that doesn't work... Try:

  void buzz(long duration, uint16_t freq) {
    if (freq > 0) {
        SET_OUTPUT(BEEPER_PIN);
          unsigned int delay = 1000000 / freq / 2;
          int i = duration * freq / 1000;
          while (i--) {
            WRITE(BEEPER_PIN, HIGH);
            delayMicroseconds(delay);
            WRITE(BEEPER_PIN, LOW);
            delayMicroseconds(delay);
           }
    }
}

If that doesn't do it... We can put the original code back in place.

[KiteLab]

@KriLL3
Did you try to define SPEAKER in configuration.h?
The frequency setting can only work for speakers (theoretically). Marlin used to misuse buzzers as speakers. That works, but the volume is reduced a lot, because the buzzer can't work on its own resonance frequency.

[KriLL3]

@KiteLab
In 1.0.2 all I had to do to use the "RepRapDiscount Full Graphic Smart Controller" was uncomment that one line "#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER" and it worked speaker and all, I assumed 1.1.0-RC2 wouldn't have changed that? Looking around in the old configuration files it seems uncommenting that line made 1.0.2 set up a bunch of other stuff like right pins, using the graphical LCD, SD card and speaker etc. The LCD controller schematic http://reprap.org/wiki/RepRapDiscount_Full_Graphic_Smart_Controller shows it has a piezo speaker connected to a transistor with one pin going to an arduino output, and other two to 5v and ground, there is no oscillator in the picture, so pwm can be used to generate a wide range of tones, not just one note.

@Roxy-3DPrintBoard
I'll try that tomorrow, family is asleep and I doubt they'd appreciate a series of fairly loud beeps.

[KiteLab]

@KriLL3
That has changed. To make it work as it did before you now have to uncomment SDSUPPORT and SPEAKER. Some call it progress.
For me, the much more bearable sound is a progress over the possibility to vary the frequency - i have a loud environment. Technically it's more correct now, as a REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER has a buzzer, not as speaker. Else you could not hear anything if SPEAKER is not defined.
SDSUPPORT is an other thing. It used to be defined in ULTIPANEL i think. ULTIPANEL is used by almost all other panels/displays. For one of the new displays it was needed to #undefine SDSUPPORT. There was/is someone around who means that should never occur, So the setting was removed from ULTIPANEL. If someone would be interested enough, he could search for the panels/displays using ULTIPANEL and readd the SDSUPPORT setting in the special sections of that displays in Conditionals.h.

[KriLL3]

Not entirely sure I'd call it progress to have to dig into the configuration to enable the features of a known LCD controller design, before the SD card and multi-frequency speaker were enabled by just specifying that you have that LCD controller... I use octopi so I don't use the SD card but still would like that feature to be accessible...

[Roxy-3D]

Not entirely sure I'd call it progress to have to dig into the configuration

Well, to be fair, it sounds like somebody got 'clever' and started using buzzers in an undocumented mode as a speaker. I can understand why that should not be encouraged.

For one of the new displays it was needed to #undefine SDSUPPORT. ... So the setting was removed from ULTIPANEL. If someone would be interested enough, he could search for the panels/displays using ULTIPANEL

Can you tell me which files I should look at?

[Wackerbarth]

"For one of the new displays it was needed to #undefine SDSUPPORT. ... So the setting was removed from ULTIPANEL." -- Well, the problem is caused by the misuse of ULTIPANEL. When it is overloaded to represent additional displays, the conflict arises. Instead, the ULTIPANEL should be replaced with a more generic display definition. Then, in Conditionals.h, ULTIPANEL can define both that more generic display and the SD support.

[Roxy-3D]

If something is not a true ULTIPANEL clone, shouldn't it have its own header.h file? If something is very close to being a true ULTIPANEL clone, could it's header.h file #include the standard ULTIPANEL file and then #undef what needs to change?

[Wackerbarth]

No, you don't undefine things. Instead, you create a definition for the common property and have both include that common element.

A Chevrolet is like a Ford EXCEPT ...
I don't start by defining a Chevrolet as a Ford.
Instead, I take those common properties and use them to define a Car.

Then, a Chevrolet is a Car plus ....
And a Ford is a Car plus ...

Until someone shows up with a Honda

Well, it, also, would be a Car plus ... except it is not built by an American manufacturer.
So we have to change something.

We start by removing the "American Manufacturer" property from "Car" since not all cars have this property. But the Ford and Chevrolet still need the property.

We can add it back by adding it explicitly to each of those definitions.
OR, we can create a new class "AmericanCar" which is a "Car" and also has the "American Manufacturer" property. And then use "AmericanCar" rather than "Car" for our Ford and Chevrolet.

Either way, we never undefine a property.

[Roxy-3D]

Well... It seems like UltiPanel should be its own thing. It is important enough to have its own files. And to have all of its features included for use.

And rather than build up a new thing that is almost an UltiPanel, it doesn't seem that bad (to me) to start with an UltiPanel and un-define the one thing that is different. From a perspective of trying to make the code understandable, that is easier for me to get my head around than looking at two big files, line by line trying to find the one difference.

[Wackerbarth]

You're thinking "flat" rather than "hierarchal". Rather than starting at the specific case (the UltiPanel), think about the more generic feature (the user control panel). The UltiPanel is just a special case of this more general class.

Just as when the biologists describe some dog, they don't start by listing every characteristic individually.
Instead, the simply say that a dog is a mammal and we automatically know that it has certain characteristics (because those characteristics are common to ALL mammals).

Similarly, we can say that each of the panels has all of the characteristics common to a panel and need specify only the distinctive components.

Now, this "inclusion" can be done either by defining a symbol for the generic class and including that symbol in the specific descriptions or, equivalently, we can use the pre-processor "#include" to pull in a file specifying those properties. In either case, we don't have "two big files" to compare. Instead, we have two small files which contain the distinctions and one larger file that contains the commonality.

[Roxy-3D]

"For one of the new displays it was needed to #undefine SDSUPPORT. ... So the setting was removed from ULTIPANEL."

Yes, but some how... The UltiPanel lost its ability to do SD Memory cards. It would seem UltiPanel's should not suffer just because some new display shows up that is close, but not really an UltiPanel.

It would seem to me the new display should fend for itself and come up with its own definitions

In a perfect world, UltiPanel's should probably get defined by pulling in this and that defination file for the various features. And the new panel could pull in most of the stuff, but just omit the SD Memory card stuff.

Right?

[KiteLab]

@Roxy-3DPrintBoard

Can you tell me which files I should look at?

I don't want to scare you, but about everything what has to do with displays, menus, pins, configuration, conditionals, sd-cards, ... . A grep for ULTIPANEL has about 150 hits in 44 files.
A somewhat challenging task, to sort that out. Worse - every change affects all kinds of displays/panels. I don't know anyone who can test more than 3 types of displays.

[Roxy-3D]

What is the display name and type that is very close to a UltiPanel but is missing the SD Memory card feature? It doesn't seem like it should be allowed to call itself an UltiPanel if it doesn't do everything an UltiPanel does.

[KiteLab]

I remembered wrong. The actual reason for the change was different.
The change: https://github.com/MarlinFirmware/MarlinDev/pull/139
The discussion before: https://github.com/MarlinFirmware/MarlinDev/issues/133
What i remembered and prepared Thinky was dealing with SDCARDDETECTINVERTED: #2144

Anyway, all panels define ether ULTIPANEL or ULTIMAKERCONTROLLER what defines ULTIPANEL. It used to be a synonym for 'has hardware capable to use menus and sd'. Now it only means 'has hardware capable to use menus' - and that is the way to go i think. Splitting off the single components from ULTIPANEL, part for part, property for property, until we get a ULTIPANEL only meaning itself. For that we'll need more abstract constructs like

#if (ENABLED(HAS_ENCODER) || ENABLED(HAS_MENU_KEYS)) && ENABLED(USE_MUNUINPUT)
  #define MENUINPUT
#endif
#if ENABLED(DISPLAY) && ENABLED(MENUINPUT) && ENABLED(USE_MENU)
  #define MENU
#endif

#if ENABLED(MENU)
  void menu() {
  ...
  }
#endif

#if ENABLED(DISPLAY) && ENABLED(USE_DISPLAY)
 void status_screen() {
  ...
#endif

It's a good thing to have #define USE_DISPLAY ON by default but only if it is sourounded by a

#if DISABLED(DO_NOT_USE_DISPLAY)
  #define USE_DISPLAY
#endif

Or how about DISPLAY_OFF?
Or in this special case SD_CARD_OFF?
Maybe that is the way to handle the capability of ULTIPANEL to handle SDSUPPORT.

For me this is a complex we should return to a bit later, but not handle it right now.

[Wackerbarth]

@KiteLab --
We seem to have two problems here.
The first is that SD card was removed from the UltiPanel, but it appears that some functionality might have been dropped.

The other is that "ULTIPANEL" appears to have been used in many places where something like "USE_MENU" would be more appropriate.

-- your comment about "USE_DISPLAY" being on by default has the right motivation. However,your code snippet is entirely wrong.

I don't see any reason why we cannot start engineering this migration in the dev branch.
We would start by doing a global replacement of ULTIPANEL with USE_MENU in all of the code (not configuration) files and have ULTIPANEL define USE_MENU by default.

From there, someone will need to look at each code instance to determine if that code applies to all of the MENU systems, or only to the specifically ULTIPANEL ones. But, the transition should not disrupt working configurations.

[AnHardt]

@ntoff
Please don't confuse things.
A piezo is a sound device using a crystal instead of an magnet.
A buzzer is a sound device having its own frequency generator.
That's completely independent from each other.

In the Adruino example they talk about.

A Piezo is nothing but an electronic device that can both be used to play tones and to detect tones.

That excludes that they could have used a buzzer. You can't use a buzzer as a microphone.

The little black cylinders (about 1x1cm) with easer a little hole in the top or covered by a sticker, on our boards can be piezo speakers or piezo buzzers. You can't distinguish them optically, if you can't read the partnumber. They look identical. But there is a simple test. Turn off SPEAKER - when you still can hear a sound you have a buzzer.
Many types of buzzers can be used like a speaker. You can feed them with a pulsed current and they will make some noise. But the result is not determined. The result is different between my red and my black RRDSCs, my RRDFGC is different an other time. The main difference is volume. Using them as a buzzer gives always a highly audible signal.

I have no concerns about using buzzers as speakers. Do it, if you want to. But don't set this as the default for all.

[Roxy-3D]

@KiteLab Thank You for all the work it took to put together that synopsis of what happened with UltiPanel and SD Memory cards!!! That was very helpful in understanding things.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.