dmfutcher / rustfm-scrobble Goto Github PK

Hi,

thank you very much for implementing this! Is it, by any means, possible to reuse your code to implement a scrobble server? If yes, would you do that?

Would be awesome to build a scrobble server so I do not have to rely on last.fm services, for example!

Either way, thanks for developing this and I hope you continue to develop it! ❤️

Investigate the potential of adding async support to the crate. Need to look at the async landscape in Rust & whether it makes sense to support it in-crate.

Example code could be improved, particularly making it clearer how it works, adding docs/comments and probably moving Scrobble type generation out of the scrobble macro (unless we want to put that into the crate?)

Scrobble succeeds but response parsing fails:

invalid type: map, expected a sequence at line 1 column 60

Easily fixed by checking if there's only one track to be scrobbled like in w4/dobble@69fb931 but might be surprising behaviour since it's a runtime failure. Caused thousands of extra scrobbles in my case.

Hi, a user of rescrobbled ran into an issue where they couldn't authenticate with Last.fm behind a proxy. I think the problem is one of two things:

1. ureq doesn't automatically pick up proxy configuration from the environment;
2. ureq doesn't support HTTPS proxies.

I've managed to fix this by replacing ureq with attohttpc in my fork. attohttpc also has a tiny footprint and is based on cURL, so in general should be reliable.

I could create a PR, but I understand if you'd rather keep ureq and fix the issue in rustfm-scrobble instead.

• Improve Usage documentation (quick code example, update authentication)
• Look at the other sections, improve or expand where useful
• Update Status section

There are no unit tests. At all. For 1.0 there should be. It's not a big crate so adding tests shoudln't be too painful. A CI pipeline to run them would be even better.

Allow modification of the Scrobble timestamp. Right now the API assumes the scrobble is to be recorded at the instant of the API call. However, it should be possible for the library user to specify the scrobble time. This feature is useful on sites like https://vinylscrobbler.com/, where the music has been listened to in the past.

Last.fm authentication requests respond with a session key, which can be used (with api secret & api key) to authenticate indefinitely.

Documentation needs expanding quite dramatically. Currently it is very barebones, see https://docs.rs/rustfm-scrobble/0.9.2/rustfm_scrobble/

For 1.0 release we want documentation for every module, struct etc. we export. The crate isn't too complicated, nor too big, so documenting shouldn't be too painful.

Last.fm API supports more Track meta-data than rustfm-scrobble currently allows (just a pair of strings; track name and artist name). See http://www.last.fm/api/show/track.updateNowPlaying for supported meta-data (the same Track data is supported on the scrobble endpoint).

New version of reqwest, v0.10 (https://github.com/seanmonstar/reqwest/releases/tag/v0.10.0), has breaking changes and will require code changes to use.

Not urgent but would like to keep up to date. Potential to help with #36.

Scrobbler::authenticate is deprecated as of v0.9.1, in favour of using authenticate_with_password, authenticate_with_token or authenticate_with_session_key.

Version 1.0 feels like a good place to remove this old method.

D:\prj\rustfm-scrobble (master -> origin)
λ cargo test
Compiling try-lock v0.2.2
Compiling crc32fast v1.2.0
Compiling regex-syntax v0.6.12
Compiling native-tls v0.2.3
Compiling percent-encoding v2.1.0
error: local variables in const fn are unstable
--> C:\Users\Андрей.cargo\registry\src\github.com-1ecc6299db9ec823\percent-encoding-2.1.0\lib.rs:72:13
|
72 | let chunk = self.mask[byte as usize / BITS_PER_CHUNK];
| ^^^^^

error: aborting due to previous error

error: Could not compile percent-encoding.
warning: build failed, waiting for other jobs to finish...
error: build failed

D:\prj\rustfm-scrobble (master -> origin)

Currently depends on Serde 0.9. Serde 1.0.x is out, rustfm-scrobble should be upgraded. There's a big change to how to write custom Deserialisers (used in dto.rs), which makes this nontrivial.

track.scrobble endpoint returns a timestamp in the response, this is missing from ScrobbleResponse struct.

Currently only Track/Artist pair are used for Scrobbles. However, this leads to inadequate tracking of played albums on Last.fm. Last.fm does not automatically match a Track & Artist name to an album name, so album play counts are incorrect for plays scrobbled through this lib.

Last.fm scrobble endpoint can accept up to 50 scrabbles in a batch. Currently rustfm-scrobble's ScrobbleBatch is currently unbounded.

In the vein of the Libz Blitz, for 1.0.0 release, meet all applicable API guidelines. https://rust-lang-nursery.github.io/api-guidelines/type-safety.html

It would be good to see Libre.fm support for this library. As Libre.fm aims to mimic Last.fm's scrobbler API, I think one solution would be to allow a custom API endpoint URI to be specifed when Scrobbler::new() is specified; this could be used to specify the Libre.fm API base URI instead, but by default, it would use Last.fm's endpoint.

I'd be glad to have a crack at implementing a PR to implement the above proposed solution, if it'll work. I'd need to test it first..

Thoughts? 😄

Scrobbles can be submitted to the Last.fm API in batches (for example adding whole albums listened to in the past, or for migrating data to a new account). Depends on #2 to make sense.

Refactor API to use a track data type to pass track data around, instead of using bare strings as params. This will allow easier implementation of #7 as well as tidy up the API.