Skip to content

[WIP] Printer information menu#4175

Closed
birkett wants to merge 8 commits intoMarlinFirmware:RCBugFixfrom
birkett:info-menu
Closed

[WIP] Printer information menu#4175
birkett wants to merge 8 commits intoMarlinFirmware:RCBugFixfrom
birkett:info-menu

Conversation

@birkett
Copy link
Contributor

@birkett birkett commented Jun 30, 2016

Adds an "About Printer" menu entry in the main menu, to show basic information about the printer and firmware configuration.

Currently untested, and incomplete.

TODO:
Add string translations for languages other than English

@jbrazio
Copy link
Contributor

jbrazio commented Jun 30, 2016

This is a good idea.
Don't forget to add print job information is the user has the option enabled in the config.

@jbrazio
Copy link
Contributor

jbrazio commented Jun 30, 2016

I just find the board name an overkill, maybe it would suffice to show the board #define ? It would be less one additional list to maintain. Or if would like to keep the pretty name, maybe the name #define would go inside each pin file having a default #define of "unknown" ?

@birkett
Copy link
Contributor Author

birkett commented Jun 30, 2016

I like the idea of having the board name visible, it is possible someone has two similar machines using different boards. The way I originally handled the #ifdef mess was a bit of a hack - your suggestion of using pins_*.h has cleaned it up significantly.

@lintz originally mentioned temperature sensors and power supply in #4152
I can't see any clean way of implementing thermistor names. Any suggestions, or skip entirely?

The print job info is on my list, but will need a getter method adding to the PrintCounter class. Any objections?


#ifdef BOARD_NAME
#undef BOARD_NAME
#endif
Copy link
Member

Choose a reason for hiding this comment

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

You can just use #undef BOARD_NAME and remove the #ifdef block.

@thinkyhead
Copy link
Member

thinkyhead commented Jun 30, 2016

It looks like you need to do a git rebase upstream/RCBugFix to bring your branch up to date.

git fetch upsteam
git checkout info-menu
git rebase upstream/RCBugFix
git push -f

@thinkyhead
Copy link
Member

thinkyhead commented Jun 30, 2016

I can't see any clean way of implementing thermistor names. Any suggestions, or skip entirely?

Actually, it's not pretty, but there is a trick you can do by using #include inline…

#define THERMISTOR_ID TEMP_SENSOR_0
#include "get_thermistor_name.h"
STATIC_ITEM(MSG_THERMISTOR " 0: " THERMISTOR_NAME);

#if TEMP_SENSOR_1 != 0
  #undef THERMISTOR_ID
  #define THERMISTOR_ID TEMP_SENSOR_1
  #include "get_thermistor_name.h"
  STATIC_ITEM(MSG_THERMISTOR " 1: " THERMISTOR_NAME);
#endif

#if TEMP_SENSOR_2 != 0
  #undef THERMISTOR_ID
  #define THERMISTOR_ID TEMP_SENSOR_2
  #include "get_thermistor_name.h"
  STATIC_ITEM(MSG_THERMISTOR " 2: " THERMISTOR_NAME);
#endif

#if TEMP_SENSOR_3 != 0
  #undef THERMISTOR_ID
  #define THERMISTOR_ID TEMP_SENSOR_3
  #include "get_thermistor_name.h"
  STATIC_ITEM(MSG_THERMISTOR " 3: " THERMISTOR_NAME);
#endif

Then your get_thermistor_name.h include file simply does:

#undef THERMISTOR_NAME
#if THERMISTOR_ID ==       -3
  #define THERMISTOR_NAME  "MAX31855"
#elif THERMISTOR_ID ==     -2
  #define THERMISTOR_NAME  "MAX6675"
#elif THERMISTOR_ID ==     -1
  #define THERMISTOR_NAME  "AD595"
#elif THERMISTOR_ID ==      1
  #define THERMISTOR_NAME  "100k"
. . .
#endif

@lintz
Copy link

lintz commented Jun 30, 2016

@birkett
For me it would be enough to see the defines such as

Board type: BOARD_RAMPS_14_EFB
Thermistor : 1
Extruders : 1
Min temp extruder: 5
Max temp extruder: 285 

Just as examples I took these. Of course printing the names rather then de defines would make the menu look nicer but not needed for me. O and thanks for taking up the task this fast :)

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't have to use inline here, as you're defining the body of the method inside the header the compiler with automatically inline this for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed on next push.

@jbrazio jbrazio added this to the 1.1.0 milestone Jun 30, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

2 times static void lcd_info_menu(); ???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cocked up the merge, fixed on next push.

@birkett
Copy link
Contributor Author

birkett commented Jul 1, 2016

@thinkyhead I figured that would be the way around it, but was unsure if you had any plans for the future.

@birkett birkett force-pushed the info-menu branch 12 times, most recently from 4d5f003 to 66ed8b3 Compare July 1, 2016 16:50
#endif //SDSUPPORT

#if ENABLED(LCD_INFO_MENU)
#ifdef PRINTCOUNTER
Copy link
Member

Choose a reason for hiding this comment

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

#if ENABLED(PRINTCOUNTER)

STATIC_ITEM(STRING_DISTRIBUTION_DATE ); // YYYY-MM-DD HH:MM
STATIC_ITEM(MACHINE_NAME ); // My3DPrinter
STATIC_ITEM(WEBSITE_URL ); // www.my3dprinter.com
STATIC_ITEM(MSG_INFO_EXTRUDERS ": " STRINGIFY(EXTRUDERS)); // Extruders: 2
Copy link
Member

Choose a reason for hiding this comment

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

Neat, but no. Don't add extra spaces within the function call.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just the () thing, you can leave the comments aligned.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 1, 2016

@birkett This is getting pretty ugly, and active PRs really need to be kept up to date, otherwise they are difficult to work on, we have to make our own copies of your branch, and then we might have to throw away your PR and make our own. It ends up being a lot of extra work for everyone. Unfortunately, we can only add PRs to your branch, we cannot rebase it without making our own copies.

Do you know how to use git to rebase and squash commits?
I can suggest some resources to help explain the process.

@birkett
Copy link
Contributor Author

birkett commented Jul 1, 2016

I'm familiar with rebase / squash. I planned on squashing down to one or two commits once it is "complete".

@thinkyhead
Copy link
Member

thinkyhead commented Jul 1, 2016

You can do whatever you want with your branch and commits, then use git push -f to publish the alterations. So, please, do frequently use…

git fetch upstream ; git rebase upstream/RCBugFix ; git push -f

…and we can adapt to your branch as it changes.

@birkett birkett force-pushed the info-menu branch 4 times, most recently from ef28c7d to 9cd3936 Compare July 2, 2016 19:28
Includes:
  *firmware version / branch / date.
  *extruder count
  *board information (name, serial details, power supply type)
  *thermistors (names, min/max temperatures)
  *printer statistics (PRINTCOUNTER details)

Thanks to @thinkyhead for contributions.
@thinkyhead
Copy link
Member

Your branch is looking a lot cleaner, thanks! Meanwhile, your concept of STATIC_ITEM_VAR won't work as it is. What we should do instead is to make STATIC_ITEM and lcd_implementation_drawmenu_static each take an additional argument — an optional string pointer. If the pointer is NULL, print normally. If it points to a string (such as the result of ftostr52sp(my_val) then append that to the current line. I will code that and post a PR for you shortly.

@birkett
Copy link
Contributor Author

birkett commented Jul 2, 2016

Thanks @thinkyhead - are there any other comments you have, or changes you would like made?
Is there any additional info that should be listed?

@thinkyhead
Copy link
Member

We can always extend it later. It's great to just have the basics ready to go.

@thinkyhead
Copy link
Member

As this is disabled by default, it's no problem to merge this shortly. It needs a tweak to scroll properly, but I will work on that and patch STATIC_ITEM later, as it is also used by the new M600 feature.

@thinkyhead thinkyhead closed this Jul 2, 2016
@birkett birkett deleted the info-menu branch July 3, 2016 08:48
drewmoseley pushed a commit to drewmoseley/Marlin that referenced this pull request Nov 8, 2023
cleanup: Remove unused `LcdCommands` state `LoadFilament`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants