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

Take apart kill() #2126

Closed
wants to merge 1 commit into from
Closed

Take apart kill() #2126

wants to merge 1 commit into from

Conversation

AnHardt
Copy link
Member

@AnHardt AnHardt commented May 20, 2015

in kill() and kill_()
to separate the message on the display.

kill() will display MSG_KILLED on the display as before and call kill_().
kill_() has all the functionality of the former kill() but the message on the display.

This change gives the possibility to use kill_() with a reason for the kill without overwriting it by MSG_KILLED.
The output on the serial is not changed.

in kill() and kill_()
to separate the message on the display.

kill() will display MSG_KILLED on the display as before and call kill_().
kill_() has all the functionality of the former kill() but the message on the display.

This change gives the possibility to use kill_() with a reason for the kill without overwriting it by MSG_KILLED.
The output on the serial is not changed.
@daid
Copy link
Contributor

daid commented May 20, 2015

Pretty sure you can come up with better function names then "kill_"

@AnHardt
Copy link
Member Author

AnHardt commented May 20, 2015

kill_without_display_massage() ?
kill_wdm() ?
I'm open for suggestions.

@CONSULitAS
Copy link
Contributor

kill_quiet
(in unix you would write kill -q

@AnHardt
Copy link
Member Author

AnHardt commented May 20, 2015

kill_quiet
The best until now. But not 100% - we are still talking to the serial line.

@CONSULitAS
Copy link
Contributor

OK.

kill_quiet_LCD ?

it is longer... but clearer?

@bkubicek
Copy link
Contributor

sorry, to ask, but why should we have more than one kill?
This can be easily done by a define statement?

Bernhard

On Wed, May 20, 2015 at 3:58 PM, CONSULitAS notifications@github.com
wrote:

OK.

kill_quiet_LCD ?

it is longer... but clearer?


Reply to this email directly or view it on GitHub
#2126 (comment)
.

@AnHardt
Copy link
Member Author

AnHardt commented May 20, 2015

@bkubicek
A #define sounds so static to me. I really don't know how i could get the wanted behavior with a define.

We call kill() for: kill button, M112, max_inactive_time - here MSG_ERR_KILLED is OK.
We call kill() from: watchdog (where i can find the comment "//TODO: This message gets overwritten by the kill() call")
We call kill() from cardreader.cpp when SD_PROCEDURE_DEPTH is hit - this could be a bit more informative.
I want to call kill() with _temp_error(), giving a reason - not overwritten by MSG_ERR_KILLED.

@AnHardt
Copy link
Member Author

AnHardt commented May 20, 2015

I could use a parameter for the message - but than we'll have MSG_ERR_KILLED at least 3 times in memory.

@daid
Copy link
Contributor

daid commented May 20, 2015

Identical PSTR() strings are pooled and combined into memory.

@AnHardt
Copy link
Member Author

AnHardt commented May 20, 2015

How about a c++ variant?

void kill(const char *);
void kill();
...
...
static void kill_quiet_LCD() {
...
{
void kill() {
  LCD_ALERTMESSAGEPGM(MSG_KILLED);
 kill_quiet_LCD();
}
void kill(const char *lcd_msg) {
  lcd_setalertstatuspgm(lcd_msg);
  kill_quiet_LCD();
}

Here at least kill_quiet_LCD() is not public

@AnHardt
Copy link
Member Author

AnHardt commented May 20, 2015

Replaced by #2130.

@AnHardt AnHardt closed this May 20, 2015
@AnHardt AnHardt deleted the kill branch May 20, 2015 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants