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

Fix host_action_notify calls passing SRAM pointers #17953

Merged
merged 1 commit into from
May 11, 2020

Conversation

Desuuuu
Copy link
Contributor

@Desuuuu Desuuuu commented May 10, 2020

The current host_action_notify function expects a PROGMEM pointer but it is passed an SRAM one in some cases (by MarlinUI::set_status for example), resulting in garbled data being printed on the serial interface. This behaviour can be observed by simply calling M117 with any string.

This PR adds a new host_action_notify_P function specifically taking a PROGMEM pointers and changes host_action_notify to take an SRAM pointer.

Calls to host_action_notify were updated to use the correct function.

Some variable types were also changed to use the PGM_P define.

Adds a separate host_action_notify_P specifically made for PROGMEM strings.
@thinkyhead
Copy link
Member

Careful, because not all const char * should be replaced with PGM_P. Only those that must be PSTRings. Other types of strings can be const char *, char * const, or just char * depending on the needs of the method. I will review this shortly to make sure everything is correct.

@thinkyhead
Copy link
Member

Thanks, it all looks good!

@thisiskeithb
Copy link
Member

This PR breaks compilation if EMERGENCY_PARSER is enabled on LPC1768 and probably other LPC-based configs. See #17955 for more details

@Desuuuu Desuuuu restored the host-actions-fix branch May 11, 2020 12:11
@Desuuuu
Copy link
Contributor Author

Desuuuu commented May 11, 2020

This is caused by the include change from MarlinConfigPre.h to MarlinConfig.h in host_actions.h (which sole purpose was to have PGM_P defined).

It's causing an error in dependencies because host_actions.h is being included in MarlinSerial.h (through e_parser.h) in multiple HAL (LPC1768 being one), while the HAL is itself included by the MarlinConfig.h include (which was not the case with MarlinConfigPre.h).

The include change should be reverted as well as the PGM_P references (or PGM_P should be defined).

vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request May 29, 2020
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
njibhu pushed a commit to njibhu/Marlin that referenced this pull request Aug 24, 2020
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
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.

3 participants