Editorial review of the V1.0 book text. #618

jvanderk · 2022-02-20T18:26:52Z

Editorial change proposals for V1.0 book text, English version only.

book/en/src/by-example/app_priorities.md

book/en/src/internals/conceptual_view.md

AfoHT

Thank you for taking the time to go through this!

I've commented on some of the smaller things, need to further study the part you've added to internal docs.

A hint, for example in the case where you had duplicate "the", you can push more commits to this branch which will update the Pull Request accordingly 👍

AfoHT · 2022-02-21T06:54:57Z

book/en/src/by-example/app_init.md

-`bare_metal::CriticalSection` token is available as `cs`) while device specific peripherals are available through
-the `core` and `device` fields of `init::Context`.
+The `init` task executes after system reset (after an optionally defined `pre-init` task/code section(?) 
+and always occurring internal RTIC


Maybe even linking to pre_init?

Don't understand the suggestion.

I interpreted the (?) after "task/code section(?)" as seeking clarification / sources, my thought was that the link could be useful here, the link in the comment.

Clear, added suggestion to text. Please check, never used that syntax.

book/en/src/by-example/app_priorities.md

AfoHT · 2022-02-21T09:28:27Z

book/en/src/by-example/hardware_tasks.md

-Any available interrupt vector should work, but different hardware might have
-added special properties to select interrupt priority levels, such as the
+Any available interrupt vector should work. Specific devices may
+allow selection of interrupt priority level, such as the


Suggested change

allow selection of interrupt priority level, such as the

assume internal use of some interrupt priority levels, such as the

Other suggestions welcome, but the main idea is that the user can't take any level as it is already being used by some other hardware component of that specific hardware.

It was not clear to me what the message of the original text was. "added properties to select ... priority" -> "to allow selection of" was intended to mean the same and break the original sentence in distinct statements. Reading your comment I would word this possibly as "Specific hardware may bind specific interrupt priorities to specific interrupt vectors outside user code control." Or something like that...

It was not clear to me what the message of the original text was. "added properties to select ... priority" -> "to allow selection of" was intended to mean the same and break the original sentence in distinct statements. Reading your comment I would word this possibly as "Specific hardware may bind specific interrupt priorities to specific interrupt vectors outside user code control." Or something like that...

^ Sounds good 👍

Amended text.

book/en/src/by-example/monotonic.md

book/en/src/SUMMARY.md

book/en/src/by-example/hardware_tasks.md

book/en/src/by-example/monotonic.md

book/en/src/by-example/tips_indirection.md

book/en/src/internals/conceptual_view.md

AfoHT · 2022-02-21T15:26:05Z

book/en/src/by-example/app_priorities.md

@@ -4,7 +4,7 @@

 The `priority` argument declares the static priority of each `task`.

-For Cortex-M, tasks can have priorities in the range `1..=(1 << NVIC_PRIO_BITS)`
+For Cortex-M, tasks can have priorities in the range `1..(1 << NVIC_PRIO_BITS)`


In Rust, ..= indicates it is an inclusive range

https://doc.rust-lang.org/std/ops/struct.RangeInclusive.html

If I recall correctly that is intentional, we can provide one more priority level by disabling the interrupts globally.

I was not aware of the rangeinclusive notation. Thx.

Maybe you fixed it, but for me it still shows as modified and = is missing.

jvanderk · 2022-02-21T21:19:41Z

I'm struggling with magit---pushing commits to jvanderk/cortex-m-rtic, but I'm not sure they are visible in the pull request.

jvanderk · 2022-03-06T23:17:09Z

Please let me know if there are still open items.

I am working on getting vigilant mode working. Apologies for 'unverified' flags.

AfoHT · 2022-04-01T08:10:10Z

book/en/src/by-example/app_priorities.md

 until task A completes execution. Thus, when task A completes task B resumes execution.

 ```text
+Task Priority


Duplicated lines?

AfoHT · 2022-04-01T11:58:53Z

book/en/src/internals/conceptual_view.md

+# Conceptual view
+
+![Conceptual execution model](./conceptual_execution_model.png)
+
+This tries to provide a high level view of how tasks are managed in
+the RTIC context. It focuses on the execution flow.
+
+At its core, RTIC leverages the NVIC subsystem of the machine to
+enable time-deterministic control of code execution.
+
+There are two queues to think about.
+
+One queue is a software queue, which is a a set of FIFOs written in
+code and managed by the RTIC framework. Individual FIFOs are
+associated with different execution priority levels.
+
+The other is a hardware-controlled queue maintained by the NVIC. If
+there are two hardware interrupts pending, the highest priority code
+will execute first: a "priority queue" at a hardware level.
+
+The NVIC operates essentially independently of the CPU, outside of
+the various masking and other registers that the CPU can use to ignore
+specific interrupt sources.
+
+(Are HW interrupts always of different priority? Does the HW maintain
+memory of lower or same priority level interrupts to process them once
+they become eligible?)
+
+So, a hardware task initiated by and bound to a hardware interrupt is
+controlled by the NVIC's queuing mechanism without direct involement
+of the processor; a software-spawned task just gets put into the task
+FIFO (at some priority level) for later execution whenever the current
+execution priority level is low enough. If a hardware or software task
+completes, RTIC checks the software queue for more tasks to run,
+eventually completing all until the queue exhausts and the IDLE task
+then takes over again.
+
+The CPU's ability to set a "minimum priority level" of hardware
+interrupts allows alignment of the software and hardware queues. This
+masks hardware-level interrupts coming in while equal or higher
+priority level software tasks are exectuting. It makes the NVIC keep
+the hardware tasks interrupt pending until the software task ends (and
+the CPU has lowered the minimum priority level of interrupt acceptance).
+
+## Hardware queue: NVIC
+
+There is a difference between a software-initiated context switch
+(which is a function call in code) and an interrupt (NVIC) initiated
+switch. When the NVIC generates an interrupt request, this physically
+breaks the execution flow of the processor.
+
+## Software queue: task dispatchers
+
+The software queues are maintained by task dispatchers. They can be
+thought of as "pre-wired" hardware tasks initiated by the NVIC.
+
+Each task dispatcher keeps a queue of tasks which are ready to
+execute; this queue is referred to as the ready queue. Spawning a
+software task consists of adding an entry to this queue and pending
+the interrupt that runs the corresponding task dispatcher. Each entry
+in this queue contains a tag (enum) that identifies the task to
+execute and a pointer to the message passed to the task.
+
+You can think of the dispatchers as just calling the SW tasks as
+functions.
+
+The ready queue is a SPSC (Single Producer Single Consumer) lock-free
+queue. The task dispatcher owns the consumer endpoint of the queue;
+the producer endpoint is treated as a resource contended by the tasks
+that can spawn other tasks.  source: [Software tasks - Real-Time
+Interrupt-driven
+Concurrency](https://rtic.rs/0.5/book/en/internals/tasks.html).
+
+## spawning
+
+Any task may influence the execution context via the "spawn" API,
+adding tasks to the queues for later execution.
+
+The defined priority of the task spawned determines the queue it lands
+in.


I think this would be best split out into a separate PR, as this is not part of the editorial review but entirely new content.

Included in this would be all the files added under internals

It would also simplify your work with git and tooling.

Makes sense. I will try to remove these changes from this branch (not sure if that is correct terminology), open a new branch against the root of this branch and open a new pull request on that. Big adventure :-)

This pull request should now only contain editorial changes. The conceptual model related text and graphics will be put into another pull request.

I am not sure if the history in this pull request will survive, if so if that is desirable or if that can be squashed out.

burrbull · 2022-04-03T16:10:19Z

mdbook supports SVG. You don't need to convert it to PNG. Just cut margins.

![Conceptual execution model](./conceptual_execution_model.svg)

jvanderk · 2022-04-03T21:42:33Z

mdbook supports SVG. You don't need to convert it to PNG. Just cut margins.
![Conceptual execution model](./conceptual_execution_model.svg)

Made change and moved files to other pull request.

AfoHT

Nice!

A few questions, but really nice in general :)

AfoHT · 2022-04-05T16:34:04Z

book/en/src/SUMMARY.md

@@ -29,6 +29,7 @@
  - [v0.4.x to v0.5.x](./migration/migration_v4.md)
  - [RTFM to RTIC](./migration/migration_rtic.md)
 - [Under the hood](./internals.md)
+  - [Conceptual execution model](./internals/conceptual_view.md)


I think this should go with the other PR :)

AfoHT · 2022-04-05T16:34:18Z

book/en/src/SUMMARY.md

+
+(XXX would it be a good idea to add a section on code organization,
+i.e. how to break a larger application into a set of files? XXX)


Same here as above

AfoHT · 2022-04-05T16:38:26Z

book/en/src/by-example/hardware_tasks.md

-At its core RTIC is using the hardware interrupt controller ([ARM NVIC on cortex-m][NVIC])
-to perform scheduling and executing tasks, and all tasks except `#[init]` and `#[idle]`
+At its core RTIC is using a hardware interrupt controller ([ARM NVIC on cortex-m][NVIC])
+to schedule and start execution of tasks. All tasks except `pre-init`, `#[init]` and `#[idle]`


Did we want some kind of link for the somewhat special pre-init, not RTIC specific, but cortex-m-rt #[pre_init]?

https://docs.rs/cortex-m-rt/latest/cortex_m_rt/attr.pre_init.html

AfoHT · 2022-05-18T07:21:36Z

Forgot that GitHub doesn't like that kind of move, Gerrit influences from work I guess xD

Will look into this during the day...

686: Book: Editorial review r=korken89 a=AfoHT Continuation of #618 Better late than never... A big thanks to `@jvanderk` ! Co-authored-by: John van der Koijk <[email protected]>

jvanderk commented Feb 20, 2022

View reviewed changes

book/en/src/by-example/app_priorities.md Outdated Show resolved Hide resolved

jvanderk commented Feb 20, 2022

View reviewed changes

book/en/src/internals/conceptual_view.md Outdated Show resolved Hide resolved

jvanderk commented Feb 20, 2022

View reviewed changes

book/en/src/internals/conceptual_view.md Outdated Show resolved Hide resolved

AfoHT reviewed Feb 21, 2022

View reviewed changes

jvanderk requested review from AfoHT and perlindgren February 21, 2022 21:16

jvanderk marked this pull request as draft February 21, 2022 21:16

jvanderk marked this pull request as ready for review February 21, 2022 22:20

AfoHT reviewed Apr 1, 2022

View reviewed changes

jvanderk marked this pull request as draft April 3, 2022 12:30

jvanderk changed the title ~~Mostly editorial review of the V1.0 book text.~~ Editorial review of the V1.0 book text. Apr 3, 2022

jvanderk marked this pull request as ready for review April 3, 2022 12:56

AfoHT reviewed Apr 5, 2022

View reviewed changes

AfoHT closed this May 18, 2022

AfoHT force-pushed the master branch from 0a1b83f to 6896749 Compare May 18, 2022 07:03

AfoHT self-assigned this May 18, 2022

AfoHT mentioned this pull request Jan 14, 2023

Book: Editorial review #686

Merged

bors bot added a commit that referenced this pull request Jan 25, 2023

Merge #686

a5e18cd

686: Book: Editorial review r=korken89 a=AfoHT Continuation of #618 Better late than never... A big thanks to `@jvanderk` ! Co-authored-by: John van der Koijk <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial review of the V1.0 book text. #618

Editorial review of the V1.0 book text. #618

jvanderk commented Feb 20, 2022 •

edited

Loading

AfoHT left a comment

AfoHT Feb 21, 2022

jvanderk Feb 22, 2022

AfoHT Mar 3, 2022

jvanderk Mar 6, 2022

AfoHT Feb 21, 2022

jvanderk Feb 21, 2022

AfoHT Mar 3, 2022

jvanderk Mar 6, 2022

AfoHT Feb 21, 2022

AfoHT Feb 21, 2022

perlindgren Feb 21, 2022

jvanderk Feb 21, 2022

AfoHT Apr 1, 2022

jvanderk Apr 2, 2022

jvanderk commented Feb 21, 2022

jvanderk commented Mar 6, 2022

AfoHT Apr 1, 2022

jvanderk Apr 2, 2022

AfoHT Apr 1, 2022

jvanderk Apr 2, 2022

jvanderk Apr 3, 2022

burrbull commented Apr 3, 2022

jvanderk commented Apr 3, 2022

AfoHT left a comment

AfoHT Apr 5, 2022

AfoHT Apr 5, 2022

AfoHT Apr 5, 2022

AfoHT commented May 18, 2022

	allow selection of interrupt priority level, such as the
	assume internal use of some interrupt priority levels, such as the


		(XXX would it be a good idea to add a section on code organization,
		i.e. how to break a larger application into a set of files? XXX)

Editorial review of the V1.0 book text. #618

Editorial review of the V1.0 book text. #618

Conversation

jvanderk commented Feb 20, 2022 • edited Loading

AfoHT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jvanderk commented Feb 21, 2022

jvanderk commented Mar 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burrbull commented Apr 3, 2022

jvanderk commented Apr 3, 2022

AfoHT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AfoHT commented May 18, 2022

jvanderk commented Feb 20, 2022 •

edited

Loading