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

Release on crates.io #15

Open
Tiwalun opened this issue Nov 18, 2022 · 12 comments
Open

Release on crates.io #15

Tiwalun opened this issue Nov 18, 2022 · 12 comments

Comments

@Tiwalun
Copy link

Tiwalun commented Nov 18, 2022

Hi,

any plans for a release on crates.io? There is a version 0.7.0 tagged in the repo, but I couldn't find it on crates.io.

We currently have a problem in probe-rs with version 0.6.3 of itm-decode (see probe-rs/probe-rs#1283), due to the indirect dependency on funty 1.2.0, which got yanked.

A new release on crates.io would be great so that we can fix this.

Thanks!

@tmplt
Copy link
Member

tmplt commented Nov 18, 2022

I'm waiting for a v0.8 release of cortex-m (rust-embedded/cortex-m#368) from which a singular structure is used IIRC. A quick fix would be to just copy the implementation verbatim for now, but I do not have a lot of time at the moment to handle this, but can review contributions. Is there no compatible funty release that supplants the yanked version?

Do note that, when a release is possible, itm-decode turns into a CLI binary and the old itm crate is updated with this repo's new implementation.

@Yatekii
Copy link

Yatekii commented Nov 18, 2022

Unfortunately there is no semver compatible update. So we need this release. Do you know what is blocking the cortex-m release? I can ping folks to have another look at your MR. Last movement was in May.

@tmplt
Copy link
Member

tmplt commented Nov 18, 2022

Do you know what is blocking the cortex-m release?

I do not. But the crate depends on cortex_m::peripheral::itm::LocalTimestampOptions and cortex_m::peripheral::scb::VectActive. However, IIRC the VectActive implementation used in the patched cortex-m now differs from cortex-m HEAD. So a v0.8 rc wouldn't be the only requirement, hence the recommendation to just copy verbatim for now.

Also, is it possible to release a crate if a feature-gated module requires unreleased code? If not, the serial module will have to be refactored.

@tmplt
Copy link
Member

tmplt commented Nov 18, 2022

Addendum: the API of itm has been redesigned in favor of iterators, but I don't think this will be a major pain point for probe-rs.

tmplt added a commit that referenced this issue Nov 18, 2022
@Yatekii
Copy link

Yatekii commented Nov 18, 2022

Can't we just make a patch release with the old codebase?

@tmplt
Copy link
Member

tmplt commented Nov 18, 2022

The old codebase is no longer maintained. But I've published a hotfix pre-release you can try out: https://crates.io/crates/itm/0.9.0-rc.1.

@Yatekii
Copy link

Yatekii commented Nov 18, 2022

Thanks!

@Yatekii
Copy link

Yatekii commented Dec 8, 2022

To be honest, the new Decoder api which consumes a reader makes this neigh impossible to use properly. You cannot access raw trace data anymore, you cannot add timeout conditions, just handling in general is basically impossible.

Is there any way to get the push() based API back?

@tmplt
Copy link
Member

tmplt commented Dec 9, 2022

I don't think it impossible to offer a push() alongside the current API. Are there any other API functions you need?

@Yatekii
Copy link

Yatekii commented Dec 9, 2022

@tmplt
Copy link
Member

tmplt commented Dec 9, 2022

For the pull_with_timestamp you can use the new iterator API.

@Yatekii
Copy link

Yatekii commented Dec 9, 2022

That's not really true. That moves the decoder. So you cannot iterate over it. So if you want to look at items in another loop that does not work. In this case you can switch it up, but for a general case the current API is not nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants