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

[BUG] HOST_STATUS_NOTIFICATIONS doesn't compile without a display anymore #26038

Closed
1 task done
jounathaen opened this issue Jul 1, 2023 · 6 comments · Fixed by #26040
Closed
1 task done

[BUG] HOST_STATUS_NOTIFICATIONS doesn't compile without a display anymore #26038

jounathaen opened this issue Jul 1, 2023 · 6 comments · Fixed by #26040

Comments

@jounathaen
Copy link

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

Hi,

When I enable HOST_STATUS_NOTIFICATIONS, but don't enable any configuration that enables some kind of display, the compilation fails.

That was most probably introduced in 574dd34, which enables the compilation of MarlinUI::host_status() in any circumstance, but the variable status_message is only available, if HAS_DISPLAY is set.

Bug Timeline

Probably since 574dd34

Expected behavior

Successful compilation 😛

Actual behavior

Compilation fails with the following error:

In file included from Marlin/src/lcd/../inc/MarlinConfigPre.h:37:0,
                 from Marlin/src/lcd/../inc/MarlinConfig.h:28,
                 from Marlin/src/lcd/marlinui.cpp:23:
Marlin/src/lcd/marlinui.cpp: In static member function 'static void MarlinUI::host_status()':
Marlin/src/lcd/marlinui.cpp:1435:50: error: 'status_message' was not declared in this scope
   TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify(status_message));
                                                  ^
Marlin/src/lcd/../inc/../core/macros.h:607:26: note: in definition of macro 'THIRD'
 #define THIRD(a,b,c,...) c
                          ^
Marlin/src/lcd/../inc/../core/macros.h:206:29: note: in expansion of macro '___TERN'
 #define __TERN(T,V...)      ___TERN(_CAT(_NO,T),V)  // Prepend '_NO' to get '_NOT_0' or '_NOT_1'
                             ^~~~~~~
Marlin/src/lcd/../inc/../core/macros.h:205:29: note: in expansion of macro '__TERN'
 #define _TERN(E,V...)       __TERN(_CAT(T_,E),V)    // Prepend 'T_' to get 'T_0' or 'T_1'
                             ^~~~~~
Marlin/src/lcd/../inc/../core/macros.h:204:29: note: in expansion of macro '_TERN'
 #define TERN_(O,A)          _TERN(_ENA_1(O),,A)     // OPTION ? 'A' : '<nul>'
                             ^~~~~
Marlin/src/lcd/marlinui.cpp:1435:3: note: in expansion of macro 'TERN_'
   TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify(status_message));
   ^~~~~
Marlin/src/lcd/marlinui.cpp:1435:50: note: suggested alternative: 'status_printf'
   TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify(status_message));
                                                  ^
Marlin/src/lcd/../inc/../core/macros.h:607:26: note: in definition of macro 'THIRD'
 #define THIRD(a,b,c,...) c
                          ^
Marlin/src/lcd/../inc/../core/macros.h:206:29: note: in expansion of macro '___TERN'
 #define __TERN(T,V...)      ___TERN(_CAT(_NO,T),V)  // Prepend '_NO' to get '_NOT_0' or '_NOT_1'
                             ^~~~~~~
Marlin/src/lcd/../inc/../core/macros.h:205:29: note: in expansion of macro '__TERN'
 #define _TERN(E,V...)       __TERN(_CAT(T_,E),V)    // Prepend 'T_' to get 'T_0' or 'T_1'
                             ^~~~~~
Marlin/src/lcd/../inc/../core/macros.h:204:29: note: in expansion of macro '_TERN'
 #define TERN_(O,A)          _TERN(_ENA_1(O),,A)     // OPTION ? 'A' : '<nul>'
                             ^~~~~
Marlin/src/lcd/marlinui.cpp:1435:3: note: in expansion of macro 'TERN_'
   TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify(status_message));
   ^~~~~

Steps to Reproduce

  1. activate HOST_ACTION_COMMANDS
  2. activate HOST_PROMPT_SUPPORT
  3. activate HOST_STATUS_NOTIFICATIONS
  4. run pio run -c platformio.ini

Version of Marlin Firmware

bugfix-2.1.x

Printer model

No response

Electronics

No response

Add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Minimal example configuration:
Configuration-host-status-notifications.zip

@jounathaen jounathaen changed the title [BUG] (HOST_STATUS_NOTIFICATIONS doesn't compile without a display anymore) [BUG] HOST_STATUS_NOTIFICATIONS doesn't compile without a display anymore Jul 1, 2023
@ellensp
Copy link
Contributor

ellensp commented Jul 2, 2023

to me this is just a invalid setting

Configuration_advance should be updated to be

  #if ALL(HOST_PROMPT_SUPPORT, HAS_STATUS_MESSAGE)
    #define HOST_STATUS_NOTIFICATIONS   // Send some status messages to the host as notifications
  #endif

so you cannot enable HOST_STATUS_NOTIFICATIONS when there is no HAS_STATUS_MESSAGE

@thisiskeithb
Copy link
Member

You can enable HOST_ACTION_COMMANDS / HOST_PROMPT_SUPPORT without an LCD, so it'd be good to fix HOST_STATUS_NOTIFICATIONS so it can also work without an LCD enabled. Then, serial-connected hosts like OctoPrint can display messages from Marlin.

I thought this was working at one point, but I've probably always had an LCD enabled when running OctoPrint.

@ellensp
Copy link
Contributor

ellensp commented Jul 2, 2023

See #22833

"This adds the host notification to the 3rd method mentioned above and adds a configuration option: HOST_STATUS_NOTIFICATIONS. If enabled, any time the UI status message is updated using any of the 3 functions mentioned above, it will also be echoed back as an action:notifiication. Without that enabled, only specific conditions trigger a notification."

With no UI this is an invalid selection

@thisiskeithb
Copy link
Member

thisiskeithb commented Jul 2, 2023

These should probably also get wrapped with HAS_STATUS_MESSAGE like they were before 574dd34:

void MarlinUI::host_notify_P(PGM_P const pstr) {
TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify_P(pstr));
}
void MarlinUI::host_notify(const char * const cstr) {
TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify(cstr));
}
void MarlinUI::host_status() {
TERN_(HOST_STATUS_NOTIFICATIONS, hostui.notify(status_message));
}

@thisiskeithb
Copy link
Member

HAS_STATUS_MESSAGE can't be used in the configs since it's defined later, but @GMagician submitted a fix in #26040.

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

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.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants