Skip to content

shell_lock: don't set CONFIG_SHELL_SHUTDOWN_ON_EXIT #19268

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

Merged
merged 2 commits into from
May 28, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Feb 10, 2023

Contribution description

We always want CONFIG_SHELL_SHUTDOWN_ON_EXIT not to be set if shell_lock us used as shell_lock will exit the shell to lock it.

Testing procedure

Run tests/shell_lock on native (or any other native app with shell_lock enabled)

Issues/PRs references

@benpicco benpicco requested review from HendrikVE and maribu and removed request for aabadie, leandrolanzieri, smlng, MichelRottleuthner and fjmolinas February 10, 2023 12:56
@github-actions github-actions bot added Area: sys Area: System Area: tests Area: tests and testing framework labels Feb 10, 2023
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: tests Area: tests and testing framework Area: sys Area: System labels Feb 10, 2023
@benpicco benpicco requested a review from kfessel February 10, 2023 12:57
@riot-ci
Copy link

riot-ci commented Feb 10, 2023

Murdock results

✔️ PASSED

c0a4acf sys/shell_lock: lock shell on EOF

Success Failures Total Runtime
6933 0 6933 10m:00s

Artifacts

@benpicco benpicco requested a review from chrysn February 13, 2023 19:49
@chrysn
Copy link
Member

chrysn commented Feb 13, 2023

In the documentation I can find on SHUTDOWN_ON_EXIT, this is about the behavior when receiving a ^D (which is often irrecoverable).

Have you tested how locking interacts with the ^D stdio closing that was observed on Ubuntu? This looks to me like there are two different concepts entangled in one effect. Like, if ^D closes stdin for good, even a shell-locked system should exit when receiving a ^D. If we want to maintain support for "we close when stdin goes bad" (which may or may not be the case, depending on how modern systems work), the exiting shell may need to report that to distinguish between "I've been locked" and "My stdin went away".

@github-actions github-actions bot added Area: sys Area: System Area: tests Area: tests and testing framework labels Feb 14, 2023
@benpicco
Copy link
Contributor Author

Ok, the shell is now also locked on ctrl-d
You can still exit native with ctrl-c

@benpicco
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request May 27, 2023
19268: shell_lock: don't set CONFIG_SHELL_SHUTDOWN_ON_EXIT r=benpicco a=benpicco



19629: cpu/stm32/periph/adc: fix setting ADC clock r=benpicco a=Enoch247

### Contribution description

The current implementation uses the core clock frequency to calculate the needed prescalar to achieve a given ADC clock frequency. This is incorrect. This patch fixes the calculation to use the correct source clock (PCKLK2 ie APB2). It also changes the defined max clock rate to use the frequency macro to improve readability.

I based on code similarity. I believe the gd32v CPU may need this same fix, but I am not familiar with that MCU.

### Testing procedure

I tested this on a nucleo-f767zi. The the MCU's reference manual is in agreement with what I have implemented here. I spot checked references manuals for a random [STM32F1](https://www.st.com/resource/en/reference_manual/cd00171190-stm32f101xx-stm32f102xx-stm32f103xx-stm32f105xx-and-stm32f107xx-advanced-arm-based-32-bit-mcus-stmicroelectronics.pdf) and [STM32F2](https://www.st.com/resource/en/reference_manual/rm0033-stm32f205xx-stm32f207xx-stm32f215xx-and-stm32f217xx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf), and they are clocked similar to the F7 I have.

### Issues/PRs references

None known.


19670: cpu/stm32: stm32f4 BRR from BSRR r=benpicco a=kfessel

### Contribution description

sometimes one wants to save one instruction :) 
just write the bits we need to write.

### Testing procedure

tests/periph/gpio_ll tests this 

### Issues/PRs references

`@maribu` might know some reference

maybe #19407

Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Joshua DeWeese <jdeweese@primecontrols.com>
Co-authored-by: Karl Fessel <karl.fessel@ovgu.de>
@bors
Copy link
Contributor

bors bot commented May 27, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented May 28, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit d58f2c8 into RIOT-OS:master May 28, 2023
@benpicco benpicco deleted the shell_lock-exit branch May 28, 2023 07:25
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants