Skip to content

anyhow Error #398

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 23, 2020
Merged

anyhow Error #398

merged 1 commit into from
Apr 23, 2020

Conversation

burrbull
Copy link
Member

@rust-highfive
Copy link

r? @Emilgardis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Nov 14, 2019
@burrbull
Copy link
Member Author

cc @therealprof

Looks like it works:

+ /home/burrbull/builds/svd2rust/release/svd2rust --target riscv -i k210.svd
ERROR svd2rust: In peripheral `FFT`
ERROR svd2rust: caused by: In register `ctrl`
ERROR svd2rust: caused by: Parsing field #0
ERROR svd2rust: caused by: In field `point`
ERROR svd2rust: caused by: Expected content in <enumeratedValues> tag, found none
ERROR svd2rust: backtrace: disabled backtrace

@therealprof
Copy link
Contributor

Sweet!

Emilgardis
Emilgardis reviewed Nov 15, 2019
View reviewed changes
Cargo.toml Outdated

[dependencies.svd-parser]
git = "https://github.com/rust-embedded/svd"
branch = "anyhow"
Copy link
Member

@Emilgardis Emilgardis Nov 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Assuming this is just for the draft.

We could use the new git+version support added for publishing crates added recently, but I'm not sure myself how to properly use it.

That would also not be backwards compatible

src/main.rs Outdated
.read_to_string(xml)
.chain_err(|| "couldn't read the SVD file")?;
.context("couldn't read the SVD file")?;
Copy link
Member

Choose a reason for hiding this comment

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

Missed this when implementing, should probably be uppercase "C"

Copy link
Member

Choose a reason for hiding this comment

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

cannot vs couldn't vs can not

I prefer cannot for this

src/main.rs Outdated
}
None => {
let stdin = std::io::stdin();
stdin
.lock()
.read_to_string(xml)
.chain_err(|| "couldn't read from stdin")?;
.context("couldn't read from stdin")?;
Copy link
Member

Choose a reason for hiding this comment

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

See above

src/generate/peripheral.rs Outdated
@@ -600,7 +600,7 @@ fn expand_cluster(cluster: &Cluster, defs: &RegisterProperties) -> Result<Vec<Re
let defs = cluster.default_register_properties.derive_from(defs);

let cluster_size = cluster_size_in_bits(cluster, &defs)
.chain_err(|| format!("Cluster {} has no determinable `size` field", cluster.name))?;
.context(format!("Cluster {} has no determinable `size` field", cluster.name))?;
Copy link
Member

Choose a reason for hiding this comment

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

Use with_context

Copy link
Member

Choose a reason for hiding this comment

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

Think there are some other places in rust-embedded/svd#98 that could do the same, but I'm sure it doesnt really matter in optimized builds.

@Emilgardis
Copy link
Member

Emilgardis commented Nov 16, 2019 •
edited
Loading

We can merge this as is, if we do we'd bump the required cargo version to publish, but not to compile (published) crate.

@burrbull
Copy link
Member Author

First we need make new release of svd-parser to remove git dependency.

@burrbull
Copy link
Member Author

By the way what MSRV version now? Can we merge #381 ?

@therealprof
Copy link
Contributor

@burrbull It changes the MSRV of the PACs to 1.37? I don't think we want to do that just yet. Bumping the MSRV of svd or svd2rust would be fine.

@burrbull burrbull marked this pull request as ready for review November 16, 2019 20:34
@burrbull burrbull requested a review from a team as a code owner November 16, 2019 20:34
@burrbull
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Nov 16, 2019
@bors
Try #398:
63d7f41
@bors
Copy link
Contributor

bors bot commented Nov 16, 2019

try

Build failed

@burrbull
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Nov 16, 2019
@bors
Try #398:
a2ba2cc
@bors
Copy link
Contributor

bors bot commented Nov 16, 2019

try

Build failed

@burrbull
Copy link
Member Author

std::backtrace::Backtrace still unstable.

@burrbull burrbull added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. label Nov 17, 2019
@burrbull
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Nov 17, 2019
@bors
Try #398:
4655c89
@bors
Copy link
Contributor

bors bot commented Nov 17, 2019

try

Build succeeded

@burrbull
Copy link
Member Author

@therealprof

Now it works on stable, but without backtrace.

If compile svd2rust on nightly, backtrace works too.

@therealprof
Copy link
Contributor

@burrbull Can you rebase this, fix the conflicts and add a ChangeLog entry?

@burrbull
Copy link
Member Author

Looks like std::backtrace::Backtrace not stable yet.

@burrbull
Copy link
Member Author

Rebased

@burrbull
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Apr 23, 2020
@bors
Try #398:
6771c10
@bors
Copy link
Contributor

bors bot commented Apr 23, 2020

try

Build succeeded:

Emilgardis
Emilgardis approved these changes Apr 23, 2020
View reviewed changes
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

thanks!
bors r+

@bors
Copy link
Contributor

bors bot commented Apr 23, 2020

Build succeeded:

@bors bors bot merged commit f7576a9 into master Apr 23, 2020
@bors bors bot deleted the anyhow branch April 23, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emilgardis Emilgardis Emilgardis approved these changes

Assignees

@Emilgardis Emilgardis

Labels
S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@burrbull @rust-highfive @therealprof @Emilgardis