Skip to content

fix docs+examples, work without reset, fixes #365

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 1 commit into from
Aug 4, 2019

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Aug 4, 2019

r? @therealprof

Add more docs as @Disasm proposed.

Restore unsafe on W::bits() I somewhere lost.

Don't generate write if reset_value is not present. #258

@burrbull burrbull requested a review from a team as a code owner August 4, 2019 06:07
@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Aug 4, 2019
src/lib.rs Outdated
//!
//! `ResetValue` trait provides `reset_value` which returns the value of the `CR2`
//! register after a reset. This value can be used to modify the
//! writable bitfields of the `CR2` register or reset it in initial state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! writable bitfields of the `CR2` register or reset it in initial state.
//! writable bitfields of the `CR2` register or reset it to its initial state.

src/lib.rs Outdated
@@ -256,26 +262,30 @@
//! }
//! ```
//!
//! ## `reset`
//!
//! `ResetValue` trait provides `reset_value` which returns the value of the `CR2`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! `ResetValue` trait provides `reset_value` which returns the value of the `CR2`
//! The `ResetValue` trait provides `reset_value` which returns the value of the `CR2`

@@ -165,16 +208,18 @@ impl<FI> R<bool, FI> {
}

///Register writer
///
///It is used as argument of closures in [`write`](Reg::write) and [`modify`](Reg::modify) methods of register
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///It is used as argument of closures in [`write`](Reg::write) and [`modify`](Reg::modify) methods of register
///Used as an argument to the closures in the [`write`](Reg::write) and [`modify`](Reg::modify) methods of the register

@@ -112,6 +151,9 @@ where
}

///Register/field reader
///
///Result of call [`read`](Reg::read) method of register.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///Result of call [`read`](Reg::read) method of register.
///Result of the [`read`](Reg::read) method of a register.

@@ -112,6 +151,9 @@ where
}

///Register/field reader
///
///Result of call [`read`](Reg::read) method of register.
///Also it can be used in [`modify`](Reg::read) method
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///Also it can be used in [`modify`](Reg::read) method
///Also it can be used in the [`modify`](Reg::read) method

@@ -100,7 +125,21 @@ where
{
///Modifies the contents of the register
///
///See [modifying](https://rust-embedded.github.io/book/start/registers.html#modifying) in book.
///Change only part of register:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///Change only part of register:
///E.g. to do a read-modify-write sequence to change parts of a register:

///or
///```ignore
///periph.reg.modify(|_, w| w
/// .field1() .bits(newfield1bits)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// .field1() .bits(newfield1bits)
/// .field1().bits(newfield1bits)

///```ignore
///periph.reg.modify(|_, w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// .field2() .set_bit()
/// .field2().set_bit()

///periph.reg.modify(|_, w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
/// .field3() .variant(VARIANT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// .field3() .variant(VARIANT)
/// .field3().variant(VARIANT)

///or write only fields you need:
///```ignore
///periph.reg.write(|w| w
/// .field1() .bits(newfield1bits)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// .field1() .bits(newfield1bits)
/// .field1().bits(newfield1bits)

///```ignore
///periph.reg.write(|w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// .field2() .set_bit()
/// .field2().set_bit()

///periph.reg.write(|w| w
/// .field1() .bits(newfield1bits)
/// .field2() .set_bit()
/// .field3() .variant(VARIANT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// .field3() .variant(VARIANT)
/// .field3().variant(VARIANT)

@@ -68,7 +79,19 @@ where
{
///Writes bits to `Writable` register
///
///See [writing](https://rust-embedded.github.io/book/start/registers.html#writing) in book.
///You can write raw bits into register:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///You can write raw bits into register:
///You can write raw bits into a register:

///```ignore
///periph.reg.write(|w| unsafe { w.bits(rawbits) });
///```
///or write only fields you need:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///or write only fields you need:
///or write only the fields you need:

@@ -42,7 +42,16 @@ where
{
///Reads the contents of `Readable` register
///
///See [reading](https://rust-embedded.github.io/book/start/registers.html#reading) in book.
///You can read contents of register in such way:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///You can read contents of register in such way:
///You can read the contents of a register in such way:

///```ignore
///let bits = periph.reg.read().bits();
///```
///or get contents of particular field of register.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///or get contents of particular field of register.
///or get the content of a particular field of a register.

@@ -55,6 +64,8 @@ where
U: Copy,
{
///Writes the reset value to `Writable` register
///
///After call this method register takes the initial state
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
///After call this method register takes the initial state
///Resets the register to its initial state

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Also should have a CHANGELOG entry mentioning the do improvements and examples. And I think the unsafe problem has snuck in in V0.15.0 because I was looking at weird "don't need unsafe here" and after removal "you do need unsafe here" warning/errors just yesterday; in that case we also need a CHANGELOG entry for this.

@burrbull
Copy link
Member Author

burrbull commented Aug 4, 2019

Done. Rebased.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Aug 4, 2019
365: fix docs+examples, work without reset, fixes r=therealprof a=burrbull

r? @therealprof 

Add more docs as @Disasm proposed.

Restore `unsafe` on `W::bits()` I somewhere lost.

Don't generate `write` if `reset_value` is not present. #258 

Co-authored-by: Andrey Zgarbul <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 4, 2019

Build succeeded

@bors bors bot merged commit fe27531 into rust-embedded:master Aug 4, 2019
@burrbull burrbull deleted the docs branch August 4, 2019 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants