WIP: Register derivedFrom and svd updates #256

jamesmunns · 2018-11-17T21:06:04Z

This PR extends #193 (edit: and #190), and also brings support for the changes made in svd-parser, primarily understanding the RegisterCluster type.

This is a very basic first cut at expanding registers with derivedFrom. This commit requires rust-embedded/svd#50

jamesmunns · 2018-11-17T21:06:13Z

bors try

bors · 2018-11-17T21:53:16Z

try

Build failed

continuous-integration/travis-ci/push

jamesmunns · 2018-11-17T21:54:56Z

Note, I believe this failure may be a regression of svd's ability to parse MSP430 SVDs. The backtrace for this failure (run locally) looks like this:

➜  other-test git:(register-derived-from) ✗ RUST_BACKTRACE=full ../target/release/svd2rust --target msp430 -i ./msp430g2553.svd
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', libcore/option.rs:355:21
stack backtrace:
   0:     0x55dbd43a19b3 - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::h76492c069e044094
                               at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1:     0x55dbd439c488 - std::sys_common::backtrace::_print::h94a018b30274f0aa
                               at libstd/sys_common/backtrace.rs:71
   2:     0x55dbd43a0b74 - std::panicking::default_hook::{{closure}}::ha3ab490579270e44
                               at libstd/sys_common/backtrace.rs:59
                               at libstd/panicking.rs:211
   3:     0x55dbd43a08dd - std::panicking::default_hook::hc3ba68bed94f8a90
                               at libstd/panicking.rs:227
   4:     0x55dbd43a11fe - std::panicking::rust_panic_with_hook::h8380fbfdb3a9ef26
                               at libstd/panicking.rs:476
   5:     0x55dbd43a0da1 - std::panicking::continue_panic_fmt::he5272a22537494ee
                               at libstd/panicking.rs:390
   6:     0x55dbd43a0c85 - rust_begin_unwind
                               at libstd/panicking.rs:325
   7:     0x55dbd43b1d0c - core::panicking::panic_fmt::he3157e9749b52890
                               at libcore/panicking.rs:77
   8:     0x55dbd43b1c3b - core::panicking::panic::h629a30fc6a0298aa
                               at libcore/panicking.rs:52
   9:     0x55dbd4305632 - <svd_parser::svd::device::Device as svd_parser::parse::Parse>::parse::h98591c7a661028b6
  10:     0x55dbd42f0a01 - svd_parser::parse::h534a94050a300c22
  11:     0x55dbd42202a7 - svd2rust::main::h34a3bf77a67818e0
  12:     0x55dbd422f3d2 - std::rt::lang_start::{{closure}}::hc6a90ef6c35166bc
  13:     0x55dbd43a0c22 - std::panicking::try::do_call::h6d19b60204e9d839
                               at libstd/rt.rs:59
                               at libstd/panicking.rs:310
  14:     0x55dbd43aeac9 - __rust_maybe_catch_panic
                               at libpanic_unwind/lib.rs:102
  15:     0x55dbd43a1533 - std::rt::lang_start_internal::h62a3cf6eecce6831
                               at libstd/panicking.rs:289
                               at libstd/panic.rs:398
                               at libstd/rt.rs:58
  16:     0x55dbd4224cc4 - main
  17:     0x7efcaa486222 - __libc_start_main
  18:     0x55dbd421319d - _start
  19:                0x0 - <unknown>

CC @ryankurte @Emilgardis, I'll dig in to this a bit, and if I find anything, I'll report it upstream to svd.

jamesmunns · 2018-11-17T23:01:45Z

I'd like @wez to review this before merging, to make sure this change is still necessary. There's some confusion described in nrf-rs/nrf52832-pac#11 (comment)

63: Make the schema field optional r=Emilgardis a=jamesmunns Some devices that have created fake SVDs, such as for the MSP430, may not have schemas listed. This fixes the error seen with https://travis-ci.org/rust-embedded/svd2rust/jobs/456451847, See also rust-embedded/svd2rust#256 (comment) Co-authored-by: James Munns <james.munns@ferrous-systems.com>

Emilgardis · 2018-11-28T13:16:48Z

ping @wez

wez · 2018-11-28T18:33:40Z

Sorry; have been swamped over the Thanksgiving holidays. I'll take a look over the weekend

wez · 2018-12-02T07:55:30Z

I tested this with https://github.com/atsamd-rs/atsamd/blob/master/update.sh and am sorry to report that it doesn't appear to correctly generate the pmux array. If you run that script and then git diff the repo, you'll see that the dim related attributes don't seem to be inherited, so rather than generating an array, it generates a sequence of wrongly sized registers that it then skips because the size sanity check doesn't pass.

It's not clear to me what the cause of this is; it looks like you pulled in the commits that made it work as-is, and then made some seemingly unrelated changes, so I suspect that something else changed in svd2rust or svd that needs to be accounted for. I didn't have time to fully dig into this

wez · 2018-12-05T16:31:08Z

Ok, I traced the issue; the SVD file declares derivedFrom as an attribute:

        <register derivedFrom="PMUX0_%s">

In my original commit to svd: wez/svd@211ae3e this was being extracted correctly, but in the current svd code this appears to have been accidentally refactored to look at a child element instead.

This patch to the svd parser is needed to fix things:

$ git diff
diff --git a/src/svd/registerinfo.rs b/src/svd/registerinfo.rs
index a075e38..6cab32f 100644
--- a/src/svd/registerinfo.rs
+++ b/src/svd/registerinfo.rs
@@ -59,7 +59,7 @@ impl RegisterInfo {
             name,
             alternate_group: tree.get_child_text_opt("alternateGroup")?,
             alternate_register: tree.get_child_text_opt("alternateRegister")?,
-            derived_from: tree.get_child_text_opt("derivedFrom")?,
+            derived_from: tree.attributes.get("derivedFrom").map(|s| s.to_owned()),
             description: tree.get_child_text("description")?,
             address_offset: tree.get_child_u32("addressOffset")?,
             size: parse::optional::<u32>("size", tree)?,

ryankurte · 2018-12-08T02:23:09Z

Ahh, looks like I got that wrong when refactoring svd, sorry!

@wez are you up for opening a PR on svd to fix that (and presumably the corresponding encode function)?, otherwise I can put it on my list.

…to register-derived-from

Register derived from (resolve merge conflicts)

Emilgardis · 2019-06-01T21:03:36Z

bors try

::Ninja Edit
Is gh notifications still down?

ryankurte · 2019-06-02T00:49:35Z

hmm bors queue says pending, but there's a crash in the log. i found a closed issue about not working with branch protection on the staging branch, but, i'm not sure why that would be an issue in this instance.

bors try

bors · 2019-06-02T00:49:36Z

try

Already running a review

Emilgardis · 2019-06-08T11:52:09Z

bors try-

bors try

bors · 2019-06-08T12:39:12Z

try

Build succeeded

continuous-integration/travis-ci/push

* Introduce a logging system. svd2rust now uses env_logger instead of printing directly to stderr. env_logger is currently configured to log messages >= info to stderr. * Fix import of logging macro. * Remove some noise in the generated code Struct initialisations don't need to repeat field names if they're equal to a local variable. Signed-off-by: Daniel Egger <daniel@eggers-club.de>

therealprof · 2019-07-15T08:47:57Z

@jamesmunns Mind rebasing this?

burrbull · 2019-07-15T08:49:18Z

Conflicts? Again?

therealprof · 2019-07-15T08:52:06Z

@burrbull Yeah, just got two PRs merged, that's the one in src/generate/register.rs. src/main.rs is the one which removed extern crate which I though you might have caught.

burrbull · 2019-07-18T16:37:48Z

cc @jamesmunns
Please, rebase this PR, or approve #314 .

@therealprof

317: Use svd-parser 0.7 and Edition 2018 r=therealprof a=burrbull Use 2 last commits from #256 : b6d2c9a and b7965db cc @therealprof Co-authored-by: James Munns <james.munns@ferrous-systems.com> Co-authored-by: Andrey Zgarbul <zgarbul.andrey@gmail.com>

This removes any observable side effect of the --nightly switch by removing the use of unions and the untagged_unions feature gate. Unions are replaced with accessor functions that return the appropriate register block reference. Here's a playground link that shows that the pointer calculation looks reasonable: https://play.integer32.com/?version=stable&mode=debug&edition=2018&gist=cd56444abc03e6781e526bbddc7082bc This commit is a breaking change. This is based on this WIP PR branch: rust-embedded#256 It implements the easiest standalone portion of this issue: rust-embedded#218 and also makes accessing the unions "safe" too, which is requested here: rust-embedded#230

@therealprof

319: register Derived from r=therealprof a=burrbull First three commits from #256 . cc @therealprof Co-authored-by: Nebojsa Sabovic <nsabovic@gmail.com> Co-authored-by: Wez Furlong <wez@wezfurlong.org> Co-authored-by: Andrey Zgarbul <zgarbul.andrey@gmail.com>

adamgreig · 2019-07-26T04:52:02Z

Closing in preference of #319 which incorporates this PR.

nsabovic and others added 6 commits August 22, 2018 16:56

First pass at implementing derivedFrom.

dd663bd

fixup semicolons

5e193ae

expand derived registers

cb134d2

This is a very basic first cut at expanding registers with derivedFrom. This commit requires rust-embedded/svd#50

point to svd master

32d8b5e

Merge branch 'master' into register-derived-from

128a308

Compiles with newest svd master

b6d2c9a

jamesmunns requested a review from a team as a code owner November 17, 2018 21:06

bors bot added a commit that referenced this pull request Nov 17, 2018

Try #256:

ebbb697

This was referenced Nov 17, 2018

New release of svd-parser? rust-embedded/svd#62

Closed

New release supporting 2018, new names, etc. nrf-rs/nrf52832-pac#11

Merged

jamesmunns mentioned this pull request Nov 17, 2018

Make the schema field optional rust-embedded/svd#63

Merged

jamesmunns changed the title ~~Register derivedFrom and svd updates~~ WIP: Register derivedFrom and svd updates Nov 17, 2018

Emilgardis added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. breaking-change and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. breaking-change labels Nov 18, 2018

wez mentioned this pull request Dec 8, 2018

derivedFrom is an attribute, not a child of <register> rust-embedded/svd#65

Merged

Emilgardis mentioned this pull request Jun 1, 2019

bump svd v0.7, rm ref #300

Closed

burrbull and others added 3 commits June 1, 2019 13:18

Merge branch 'master' of https://github.com/rust-embedded/svd2rust in…

70c885b

…to register-derived-from

rm ref

b7965db

Merge pull request #301 from burrbull/register-derived-from

e76f2c2

Register derived from (resolve merge conflicts)

Emilgardis previously approved these changes Jun 1, 2019

View reviewed changes

bors bot added a commit that referenced this pull request Jun 8, 2019

Try #256:

ef9b617

ryankurte previously approved these changes Jun 10, 2019

View reviewed changes

burrbull mentioned this pull request Jul 12, 2019

Register derivedFrom and ... (resolve conflicts) #308

Closed

therealprof dismissed stale reviews from ryankurte and Emilgardis via fbc831f July 15, 2019 08:43

burrbull mentioned this pull request Jul 16, 2019

Add support for reading and writing to the Backup Data Register (#83). stm32-rs/stm32f1xx-hal#86

Merged

burrbull mentioned this pull request Jul 19, 2019

Use svd-parser 0.7 and Edition 2018 #317

Merged

burrbull mentioned this pull request Jul 19, 2019

register Derived from #319

Merged

burrbull mentioned this pull request Jul 19, 2019

Close issues #321

Closed

adamgreig closed this Jul 26, 2019

therealprof deleted the register-derived-from branch July 26, 2019 06:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Register derivedFrom and svd updates #256

WIP: Register derivedFrom and svd updates #256

jamesmunns commented Nov 17, 2018 •

edited

Loading

jamesmunns commented Nov 17, 2018

bors bot commented Nov 17, 2018

jamesmunns commented Nov 17, 2018

jamesmunns commented Nov 17, 2018

Emilgardis commented Nov 28, 2018

wez commented Nov 28, 2018

wez commented Dec 2, 2018

wez commented Dec 5, 2018

ryankurte commented Dec 8, 2018

Emilgardis commented Jun 1, 2019 •

edited

Loading

ryankurte commented Jun 2, 2019

bors bot commented Jun 2, 2019

Emilgardis commented Jun 8, 2019

bors bot commented Jun 8, 2019

therealprof commented Jul 15, 2019

burrbull commented Jul 15, 2019

therealprof commented Jul 15, 2019

burrbull commented Jul 18, 2019

adamgreig commented Jul 26, 2019

WIP: Register derivedFrom and svd updates #256

WIP: Register derivedFrom and svd updates #256

Conversation

jamesmunns commented Nov 17, 2018 • edited Loading

jamesmunns commented Nov 17, 2018

bors bot commented Nov 17, 2018

try

Build failed

jamesmunns commented Nov 17, 2018

jamesmunns commented Nov 17, 2018

Emilgardis commented Nov 28, 2018

wez commented Nov 28, 2018

wez commented Dec 2, 2018

wez commented Dec 5, 2018

ryankurte commented Dec 8, 2018

Emilgardis commented Jun 1, 2019 • edited Loading

ryankurte commented Jun 2, 2019

bors bot commented Jun 2, 2019

try

Emilgardis commented Jun 8, 2019

bors bot commented Jun 8, 2019

try

Build succeeded

therealprof commented Jul 15, 2019

burrbull commented Jul 15, 2019

therealprof commented Jul 15, 2019

burrbull commented Jul 18, 2019

adamgreig commented Jul 26, 2019

jamesmunns commented Nov 17, 2018 •

edited

Loading

Emilgardis commented Jun 1, 2019 •

edited

Loading