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.

Sign up for GitHub

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

Jump to bottom

native: allow for native to be resetable via SIGUSR1 #12517

Merged
merged 2 commits into from
Dec 3, 2019
Merged

native: allow for native to be resetable via SIGUSR1 #12517

merged 2 commits into from
Dec 3, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Oct 20, 2019

Contribution description

Based on the discussion sparked by this comment #12448 (comment) I decided to provide a simple PoC of how to make native resetable.

Testing procedure

Start a native instance in one terminal (application is arbitrary), find out its PID in another terminal, and then use the PID to reset e.g.:

$ pgrep -f native/hello-world
96937
$ DEBUG_ADAPTER_ID=96937 make -C examples/hello-world reset

The native instance should then reboot (noticeable by a big fat !! REBOOT !! printed)

Issues/PRs references

#12448 (comment)

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: native Platform: This PR/issue effects the native platform Area: boards Area: Board ports State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. labels Oct 20, 2019
@miri64 miri64 added this to the Release 2020.01 milestone Oct 20, 2019
@miri64 miri64 mentioned this pull request Oct 20, 2019
Open
9 tasks
@fjmolinas
Copy link
Contributor

Test, works as advertised.

LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-pr-12517)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.


                !! REBOOT !!

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-pr-12517)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.

@smlng
Copy link
Member

smlng commented Dec 3, 2019

on FreeBSD I get:

~/devel/RIOT/examples/hello-world/bin/native/hello-world.elf: native_irq_handler: ignoring SIGUSR1

@smlng
Copy link
Member

smlng commented Dec 3, 2019

comes from here https://github.com/RIOT-OS/RIOT/blob/master/cpu/native/irq_cpu.c#L266

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/startup.c Outdated
@@ -577,6 +586,10 @@ __attribute__((constructor)) static void startup(int argc, char **argv, char **e
periph_init();
board_init();

/* should not error as SIGUSR is a valid error code and EINVAL is the only
* specified error code => don't check return value */
signal(SIGUSR1, reset_handler);
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to (should) use: https://github.com/RIOT-OS/RIOT/blob/master/cpu/native/irq_cpu.c#L412 instead

Copy link
Member

Choose a reason for hiding this comment

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

or additionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Might be.

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/startup.c Outdated
@@ -390,6 +392,13 @@ extern init_func_t __init_array_start;
extern init_func_t __init_array_end;
#endif

static void reset_handler(int signo)
Copy link
Member

Choose a reason for hiding this comment

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

I changed this to

static void _reset_handler(void)
{
    pm_reboot();
}

and added:

register_interrupt(SIGUSR1, _reset_handler);

below and it works on FreeBSD, the signal(...) is not needed

@smlng
Copy link
Member

smlng commented Dec 3, 2019

with the proposed changes it looks good:

/devel/RIOT/examples/hello-world/bin/native/hello-world.elf
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-HEAD)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.


                !! REBOOT !!

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.01-devel-194-g943fd-HEAD)
Hello World!
You are running RIOT on a(n) native board.
This board features a(n) native MCU.

@miri64
Copy link
Member Author

miri64 commented Dec 3, 2019

I will apply the changes @smlng proposed and retest on Linux.

@miri64
Copy link
Member Author

miri64 commented Dec 3, 2019

I will apply the changes @smlng proposed and retest on Linux.

Done and still works on Linux. Thanks for testing on FreeBSD, @smlng!

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/include/native_internal.h Outdated
*
* @see e.g. https://www.freebsd.org/cgi/man.cgi?query=signal&section=3
*/
typedef void (*real_sig_t) (int);
Copy link
Member

Choose a reason for hiding this comment

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

not needed anymore, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah, sorry.... still on my first coffee ^^"

Copy link
Member

Choose a reason for hiding this comment

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

enjoy that first ☕️

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/include/native_internal.h Outdated
@@ -121,6 +128,7 @@ extern int (*real_setitimer)(int which, const struct itimerval
*__restrict value, struct itimerval *__restrict ovalue);
extern int (*real_setsid)(void);
extern int (*real_setsockopt)(int socket, ...);
extern real_sig_t (*real_signal)(int sig, real_sig_t handler);
Copy link
Member

Choose a reason for hiding this comment

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

same here?

smlng
smlng reviewed Dec 3, 2019
View reviewed changes
cpu/native/syscalls.c Outdated
@@ -87,6 +87,7 @@ int (*real_setitimer)(int which, const struct itimerval
*restrict value, struct itimerval *restrict ovalue);
int (*real_setsid)(void);
int (*real_setsockopt)(int socket, ...);
real_sig_t (*real_signal)(int sig, real_sig_t handler);
Copy link
Member

Choose a reason for hiding this comment

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

also not needed anymore?

@smlng smlng added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Dec 3, 2019
@smlng
Copy link
Member

smlng commented Dec 3, 2019

when done, please squash and trigger Murdock please

@miri64
Copy link
Member Author

miri64 commented Dec 3, 2019

Cleaned out now unnecessary changes and squashed.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 3, 2019
smlng
smlng approved these changes Dec 3, 2019
View reviewed changes
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

tested ACK: Linux and FreeBSD

@kaspar030
Copy link
Contributor

tested ACK: Linux and FreeBSD

I guess FreeBSD implies OSX?

@smlng
Copy link
Member

smlng commented Dec 3, 2019

RIOT does not work on latest macOS, bc the latter is 64bit only now - so no macOS right now

@cgundogan
Copy link
Member

RIOT does not work on latest macOS, bc the latter is 64bit only now - so no macOS right now

@x3ro are you still working on a 64-bit native port? (:

@miri64
Copy link
Member Author

miri64 commented Dec 3, 2019

tested ACK: Linux and FreeBSD

I guess FreeBSD implies OSX?

There is I think twenty years of divergence between FreeBSD and Darwin ^^, but okay :-)

@miri64 miri64 merged commit 2f74d9d into RIOT-OS:master Dec 3, 2019
@miri64 miri64 deleted the native/enh/reset-command branch December 3, 2019 09:56
@miri64 miri64 mentioned this pull request Dec 3, 2019
Closed
@miri64 miri64 mentioned this pull request Feb 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smlng smlng smlng approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees
No one assigned
Labels
Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines State: demonstrator State: This PR (loosely) demonstrates a concept and is not necessarily meant to be merged. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2020.01
Development

Successfully merging this pull request may close these issues.
5 participants
@miri64 @fjmolinas @smlng @kaspar030 @cgundogan