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

sys/shell: don't include suit command by default #19294

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Feb 21, 2023

Contribution description

The suit command is pretty expensive as it calls suit_worker_trigger() which starts the SUIT worker thread.
If this function is not called otherwise, this will pull in the (quite substantial) suit worker thread stack.

Testing procedure

If your application calls suit_handle_url() directly (doesn't make use of the worker thread) this saves 3 * THREAD_STACKSIZE_LARGE Bytes of RAM.

Issues/PRs references

@github-actions github-actions bot added Area: examples Area: Example Applications Area: sys Area: System labels Feb 21, 2023
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 21, 2023
benpicco added a commit to ML-PA-Consulting-GmbH/RIOT that referenced this pull request Feb 21, 2023
@riot-ci
Copy link

riot-ci commented Feb 21, 2023

Murdock results

✔️ PASSED

f3aa2ac examples/suit_update: enable suit command

Success Failures Total Runtime
6865 0 6865 11m:57s

Artifacts

@maribu
Copy link
Member

maribu commented Feb 21, 2023

bors merge

@benpicco
Copy link
Contributor Author

Ah no hurry, we can fit more into the merge train

bors bot added a commit that referenced this pull request Feb 21, 2023
19294: sys/shell: don't include suit command by default r=maribu a=benpicco



Co-authored-by: Benjamin Valentin <[email protected]>
@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 21, 2023

Already running a review

@benpicco
Copy link
Contributor Author

bors cancel
bors merge

@benpicco
Copy link
Contributor Author

bors ping

@bors
Copy link
Contributor

bors bot commented Feb 21, 2023

pong

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 21, 2023

Already running a review

@benpicco
Copy link
Contributor Author

bors cancel

@bors
Copy link
Contributor

bors bot commented Feb 21, 2023

Canceled.

@benpicco
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Feb 21, 2023
18392: drivers/servo: reimplement with high level interface r=benpicco a=maribu

### Contribution description

The previous servo driver didn't provide any benefit over using PWM directly, as users controlled the servo in terms of PWM duty cycles. This changes the interface to provide a high level interface that abstracts the gory PWM details.

In addition, a SAUL layer and auto-initialization is provided.

### Testing procedure

The test application provides access to the servo driver via the `saul` shell command.

```
> saul
2022-08-02 22:12:31,826 # saul
2022-08-02 22:12:31,827 # ID	Class		Name
2022-08-02 22:12:31,830 # #0	ACT_SWITCH	LD1(green)
2022-08-02 22:12:31,832 # #1	ACT_SWITCH	LD2(blue)
2022-08-02 22:12:31,834 # #2	ACT_SWITCH	LD3(red)
2022-08-02 22:12:31,837 # #3	SENSE_BTN	B1(User button)
2022-08-02 22:12:31,838 # #4	ACT_SERVO	servo
> saul write 4 0
2022-08-02 22:12:41,443 # saul write 4 0
2022-08-02 22:12:41,445 # Writing to device #4 - servo
2022-08-02 22:12:41,447 # Data:	             0 
2022-08-02 22:12:41,450 # [servo] setting 0 to 2949 (0 / 255)
2022-08-02 22:12:41,453 # data successfully written to device #4
> saul write 4 256
2022-08-02 22:12:45,343 # saul write 4 256
2022-08-02 22:12:45,346 # Writing to device #4 - servo
2022-08-02 22:12:45,347 # Data:	           256 
2022-08-02 22:12:45,351 # [servo] setting 0 to 6865 (255 / 255)
2022-08-02 22:12:45,354 # data successfully written to device #4
```

Each write resulted in the MG90S servo that I connected to move to the corresponding position.

### Issues/PRs references

19292: sys/phydat: Fix unit confusion r=benpicco a=maribu

### Contribution description

Previously, `UNIT_G` was used for g-force with the correct symbol `g`, `UNIT_GR` for gram (as in kilogram) with the incorrect symbol `G` (which would be correct for Gauss), and `UNIT_GS` for Gauss with symbol `Gs` (which is an alternative correct symbol).

To avoid confusion between G-Force, Gauss, and Gram the units have been renamed to `UNIT_G_FORCE`, `UNIT_GRAM`, and `UNIT_GAUSS`. In addition, gram now uses the correct symbol `g`; which sadly is the same as for g-force. But usually there is enough context to tell them apart.

### Testing procedure

Green CI

### Issues/PRs references

None

19294: sys/shell: don't include suit command by default r=benpicco a=benpicco



19295: gcoap: Finish the gcoap_get_resource_list_tl -> gcoap_get_resource_list renaming r=benpicco a=chrysn

### Contribution description

In #16688, an argument was added to the `gcoap_get_resource_list` function by creating a new function `gcoap_get_resource_list_tl` with a deprecation and roll-over plan.

This plan has not been acted on so far.

This PR shortens the original plan by just adding the argument to `gcoap_get_resource_list` and removing `gcoap_get_resource_list_tl` in a single go. The rationale for this deviation is that while it's a public API, its only two practical consumers are the (built-in) well-known/core implementation, and the (built-in) CoRE Resource Directory (cord) endpoint. Moreover, a further change to this API (switching over to `coap_block_slicer_t`) is expected to happen within this release cycle, which would take something like 4 total releases to get through otherwise, which is unrealistic for an API that there are no known external users of.

A second commit clean up ToDo items (in the changed function's documentation) that referred to a IETF draft that has long been abandoned by the CoRE WG.

### Testing procedure

Plain inspection and CI passing should suffice.

### AOB

There is a second analogous pair left over from #16688, `gcoap_req_send` / `gcoap_req_send_tl`. As that *is* expected to be used widely, I prefer not to mix these two concerns, and get the present one through without unnecessary hold-up.

Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
Co-authored-by: chrysn <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 21, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Feb 22, 2023

Build succeeded:

@bors bors bot merged commit cf540a2 into RIOT-OS:master Feb 22, 2023
@benpicco benpicco deleted the shell_cmd_suit branch February 22, 2023 08:13
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR 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.

4 participants