Alxandr / SpotiFire

A project to make a SpotifyClient in C#
http://nudoc.azurewebsites.net/SpotiFire
40 stars 19 forks source link

Implemented connectionstate_updated and connection_error callbacks #29

Closed ChrisBrandhorst closed 11 years ago

ChrisBrandhorst commented 11 years ago

I added the connectionstate_updated callback and implemented the connection_error callback, so we can track online/offline status, and act on connection interruptions.

Alxandr commented 11 years ago

You planning on implementing more events or should I just go ahead and merge? Oh, and great job by the way.

ChrisBrandhorst commented 11 years ago

Sure, I can do that. Question about that, though:

Why are you using statements like:

TP1(Error, SESSION, Session::logged_in, ENUM(Error, error));

Instead of the (imho) more readable:

SESSION->logged_in(ENUM(Error, error));

?

Because in the former case, you can't pass along references, but in the latter case you can.

Alxandr commented 11 years ago

TP{N} runs the given function on the thread-pool (in other word, it doesn't block). This means that even if you do something stupid in an event-handler (like while(true){}), libspotify will continue working (albeit a bit worse because you have a thread using 100% of a core on your CPU). The only exception is the music_delivered which needs feedback (number of frames consumed), which has to be run synchronously.

ChrisBrandhorst commented 11 years ago

Check, I got it working with passing along a referenced string (me was stupid...)

Here we are: all callbacks implemented and catchable by the application (with the exception of logged_in and logged_out, but these can be captured through connectionstate_changed).

For now, I put the credentials blob (a string) in the SessionEventArgs#Message property. If you think it should be a separate EventArgs object, let me know or fix it yourselves :-)

Alxandr commented 11 years ago

Good lord, somebody buy this man a beer :)

Also, I don't want LoggedIn and LoggedOut, for as far as I know they cannot (may be wrong about this) be invoked without first self invoking Session::LogIn or Session::LogOut, and they return Tasks.

Also, I'm planning on writing an audio-buffer so that I can hide away the MusicDelivered and the BufferStatus events because I mainly think that makes things harder for people. I'm thinking of stealing (more or less) NAudio's buffer-implementation.

Also, if you like seriously have nothing better to do (or are just plain old awesome), feel free to drop examples/documentation in the xml-format. See https://github.com/Alxandr/SpotiFire/blob/master/SpotiFire.SpotifyLib/Spotify.cs for example. The documentation found at http://nudoc.azurewebsites.net/SpotiFire is automatically generated from the XML-documentation.

ChrisBrandhorst commented 11 years ago

Why, thank you :-)

I thought so about the LoggedIn and LoggedOut.

Disclaimer: I haven't tested all the new callbacks thoroughly yet, but expect to come across them in the following stages of the development of my app. I did try adding the StartPlayback in your TestClient project, but it wasn't called. Code-wise it's exactly the same as ConnectionstateUpdated, which I confirmed works. So this leads me to thinking StartPlayback is not used as you would think, or not used at all anymore (?)

For my own project, I'm (once again) building the Spotify part, so the coming weeks I'll be fiddling more with the lib, and will try to add to it were I can.

Alxandr commented 11 years ago

IIRC you are right about the StartPlayback event. That's one of the reasons I would like to create a internal buffer. Using the events StopPlayback and calls to PlayTrack it should (theoretically) be able to hide them away and generate better (and more logical) events. I think StartPlayback is from after pause or something, but I'm not sure. If you figure it out, please do add it to the doc-string.

Alxandr commented 11 years ago

Oh, and another thing. Don't create public classes without a namespace -.-

I've been hunting down a bug in my documentation-generator now for an hour -.-

ChrisBrandhorst commented 11 years ago

Did I do that? Where did I do that?

Alxandr commented 11 years ago

Nah, seems it was't you. Was some other guy. The ErrorExtensions-class was not put in a namespace. Just because you can do that in C++ doesn't mean you should ^^

With great power comes great responsibility and all that xD