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]: RepRapFirmware 3.5.0-rc.1 and Screens 12864 #909

Closed
2 of 25 tasks
X-Dron opened this issue Sep 13, 2023 · 14 comments
Closed
2 of 25 tasks

[Bug]: RepRapFirmware 3.5.0-rc.1 and Screens 12864 #909

X-Dron opened this issue Sep 13, 2023 · 14 comments
Assignees
Labels
bug Bug that has been reproduced
Milestone

Comments

@X-Dron
Copy link

X-Dron commented Sep 13, 2023

Duet Forum Discussion Thread

https://forum.duet3d.com/categories

Which Duet products are you using?

  • Duet2-Wifi
  • Duet2-Ethernet
  • Duet Expansion Breakout Board
  • Duex2
  • Duex5
  • Duet2-Maestro
  • Maestro Dual Driver Expansion
  • Duet3-6HC
  • Duet3-3HC
  • Duet3-1XD
  • Duet3-1LC
  • Duet3-Tool Distribution Board
  • Duet3-Mini5+
  • Duet3-Mini2+
  • Raspberry Pi or other SBC
  • SmartEffector
  • Magnetic Filament Sensor
  • Laser Filament Sensor
  • PT100 Daughterboard
  • Thermocouple Daughterboard
  • PanelDue
  • Other
  • None

Firmware Version

RepRapFirmware 3.5.0-rc.1

Duet Web Control Version

DWC 3.5.0-rc.1

Are you using a Single Board Computer (RaspberryPi) with your Duet?

  • Yes I use a SBC.
  • No I do not use a SBC.

Please upload the results of sending M122 in the gcode console.

M122 Report

Please upload the content of your config.g file.

Config.g

Please upload the content of any other releveant macro files.

No response

Details specific to your printer.

No response

Links to additional info.

No response

What happened?

Hi, after these changes:
M98 now requires the P parameter to be a quoted string. Previously, in standalone mode an unquoted string was permitted.
M98 now always requires the P parameter to be a quoted string.
starting macros from screens 12864 does not work
Does not work:
button R0 C0 W63 F0 H1 T"Auto Level" A"M98 P0:/menu/scripts/autoLevel.g"
Does not work too:
button R0 C0 W63 F0 H1 T"Auto Level" A"M98 P#0" L"/menu/scripts/autoLevel.g"
In RRF 3.5.0-beta.4 everything works correctly

@X-Dron X-Dron added the bug Bug that has been reproduced label Sep 13, 2023
@T3P3
Copy link
Contributor

T3P3 commented Sep 13, 2023

can you try using:

button R0 C0 W63 F0 H1 T"Auto Level" A""M98 P0:/menu/scripts/autoLevel.g""

@X-Dron
Copy link
Author

X-Dron commented Sep 13, 2023

Does not work (((
Error loading menu
File: bedLevel
Line 21 collum 41
Bad arg letter

@T3P3
Copy link
Contributor

T3P3 commented Sep 13, 2023

apologies, try

button R0 C0 W63 F0 H1 T"Auto Level" A"""M98 P0:/menu/scripts/autoLevel.g"""

@dc42
Copy link
Collaborator

dc42 commented Sep 13, 2023

Should be this I think:

button R0 C0 W63 F0 H1 T"Auto Level" A"M98 P""0:/menu/scripts/autoLevel.g"""

@X-Dron
Copy link
Author

X-Dron commented Sep 13, 2023

Should be this I think:

button R0 C0 W63 F0 H1 T"Auto Level" A"M98 P""0:/menu/scripts/autoLevel.g"""

Does not work (((
There is an empty command button and the command is not executed.

@oliof
Copy link
Contributor

oliof commented Sep 13, 2023 via email

@X-Dron
Copy link
Author

X-Dron commented Sep 13, 2023

that's missing a third " between P and 0 ... Gesendet von ProtonMail mobile

-------- Original-Nachricht -------- Am 13. Sept. 2023, 16:29, schrieb X-Dron :

Should be this I think: > > button R0 C0 W63 F0 H1 T"Auto Level" A"M98 P""0:/menu/scripts/autoLevel.g""" Does not work ((( There is an empty command button and the command is not executed. — Reply to this email directly, [view it on GitHub](#909 (comment)), or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

Does not work (((


Error loading menu
File: bedLevel
Line 21 collum 48
Bad arg letter


See src/Display/Menu.cpp
const char *Menu::ParseMenuLine(char * const commandWord) noexcept

		case 'T':
		case 'L':
		case 'A':
		case 'I':
			if (*args != '"')
			{
				errorColumn = (args - commandWord) + 1;
				return "Missing string arg";
			}
			++args;
			((ch == 'T') ? text : (ch == 'A') ? action : (ch == 'I') ? dirpath : fname) = args;
			while (*args != '"' && *args != 0)
			{
				++args;
			}
			if (*args == '"')
			{
				*args = 0;
				++args;
			}
			break;

		default:
			errorColumn = (args - commandWord);
			return "Bad arg letter";

@dc42 dc42 added this to the 3.5.0 milestone Sep 14, 2023
@dc42 dc42 assigned dc42 and unassigned x0rtrunks Sep 14, 2023
@X-Dron
Copy link
Author

X-Dron commented Sep 14, 2023

I think the best way is to change
ButtonMenuItem::Select(const StringRef& cmd)
in
/src/Display/ButtonMenuItem.cpp

bool ButtonMenuItem::Select(const StringRef& cmd) noexcept
{
	const int nReplacementIndex = StringContains(command, "#0");
	if (-1 != nReplacementIndex)
	{
		cmd.copy(command, nReplacementIndex);
		cmd.cat(m_acFile);
	}
	else
	{
		cmd.copy(command);
		if (StringEqualsIgnoreCase(command, "menu") && strlen(m_acFile) != 0)
		{
			// For backwards compatibility, if 'menu' is used without any parameters, use the L parameter as the name of the menu file
			cmd.cat(' ');
			cmd.cat(m_acFile);
		}
	}

	return true;
}

@dc42
Copy link
Collaborator

dc42 commented Sep 15, 2023

I will look at this in more detail next week, but my current plan is to change ParseMenuLine to recognise a pair of adjacent double quote characters within a string literal as representing and embedded double quote character.

@X-Dron
Copy link
Author

X-Dron commented Sep 17, 2023

I will look at this in more detail next week, but my current plan is to change ParseMenuLine to recognise a pair of adjacent double quote characters within a string literal as representing and embedded double quote character.

I think it's better to leave the menu file syntax as is. So there is no need to update the parser. It parses the command and file path correctly. The G-code command generation syntax needs to be updated (ButtonMenuItem::Select).

@dc42
Copy link
Collaborator

dc42 commented Sep 24, 2023

The problem lies in the parser. Given the argument A"M98 P""test.g""" it sets the string argument to"M98 P", then it parses "test.g" and treats that as an implicit T parameter (changing the button text), then it parses the final "" and treats that as an implicit T parameter (changing the button text to an empty string). So the parser needs to be fixed.

RRF in standalone mode used to allow the P argument of M98 not to be quoted e.g. A"M98 Ptest.g" but that causes problems if you want to pass parameters to the macro.

I have fixed the parser so that a line such as button C64 T" Macro " A"M98 P""test.g""" now works.

@dc42 dc42 closed this as completed Sep 24, 2023
@pfn
Copy link

pfn commented Nov 15, 2023

@dc42 This is a bad solution IMO as it introduces inconsistency into the menus language, e.g. for the files command A"M98 P#0" does not need the #0 argument quoted, whereas for button it is necessary. Has the change been made in a way that files must also be modified such that #0 is quoted?

In 3.5rc1, files R15 C5 F0 N4 I"0:/macros/" A"M98 P#0" works, but button R25 C64 F0 H0 V5 T"- Save Z" A"M98 P#0" L"/macros/Calibration/Save Z Offset" does not.

@dc42
Copy link
Collaborator

dc42 commented Nov 15, 2023

#0 expands to a quoted string, therefore it does not need to be quoted.

@pfn
Copy link

pfn commented Nov 16, 2023

It (#0) isn't currently working in rc1, so it is fixed in rc1+?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that has been reproduced
Projects
None yet
Development

No branches or pull requests

6 participants