The tests, for example, will create a playlist named “rspec” and another named “omgwtfbbq”. Both of these are rarely (if ever) added to the container on the Spotify servers, nor actually uploaded.

When a playlist has been created it should exist everywhere, not just in local memory.

It should support Enumerable module, but also needs to implement some methods like #at and possibly #slice.

How to do this in a general way I have no idea, but I'll look at it tomorro'.

Given this file in the path spec/bug.rb:
$: << File.expand_path('../../lib', FILE)
require 'hallon'
require_relative 'support/config'

session = Hallon::Session.instance(Hallon::APPKEY)
puts "Handler: #{session.handler.inspect}"

This will segfault on line #6 when inspecting the handler. I do not yet know why. Here’s some more info:

• Handler is made and set at session.c:105
• Session#handler returns whatever the handler is set to
• Macro to build handler is in common.h

Everything is written within the same file. This is bad for extensibility and needs to be adressed.

Session, Image… um, some more objects. Playlists, for sure.

Also see #43.

Turns out Playlists, Playlist Containers and… Searches not only allow you to retrieve a specific object by its’ index, but also some status information on it and even allow you to modify fields.

Say, for Playlists, we have sp_track *sp_playlist_track_message (sp_playlist *playlist, int index). Thing is, we also have:

int sp_playlist_track_create_time (sp_playlist *playlist, int index);
sp_user * sp_playlist_track_creator (sp_playlist *playlist, int index);
bool sp_playlist_track_seen (sp_playlist *playlist, int index);
sp_error sp_playlist_track_set_seen (sp_playlist *playlist, int index, bool seen);
const char * sp_playlist_track_message (sp_playlist *playlist, int index);

It is clear that Hallon need to support these (or similar) API calls:

playlist.track[0].name # called on track
playlist.track[0].artist   # called on track
playlist.track[0].creator # called through playlist_track_creator
playlist.track[0].seen = true # called through playlist
playlist.track[0].message # called through playlist

I am thinking some kind of proxy object where you explicitly define your extra methods, and any other methods fall through to the underlying object at that position. Oh well, that’s for another day.

It would do the same thing as Session#login, except it’d be blocking and wait for an error or successful login.

Might also be worth it adding a Session#relogin!, too, as it’s pretty much the same functionality.

The reasoning behind this change is that you pretty much always want to log in to Spotify when using libspotify. It would be silly and tedious to copy-paste four lines of code just to log in if you need no special functionality.

http://travis-ci.org/#!/Burgestrand/Hallon/builds/113014

This is because Hallon uses submodules for development, and Travis does not check them out.

Travis has an issue about this: travis-ci/travis-worker#5

After adding the rest of the Session callbacks, I noticed the specs sometimes deadlock. All occasions so far are on Hallon::Link.

I believe the cause of this is that ruby maintains GVL during the libspotify function calls on Hallon::Link, and that these function calls do some callback calling behind the curtains.

Some bugtracking is required.

Image.new takes the Session as a parameter, but defaults to Session.instance if none is given.

AlbumBrowse.new, on the other hand, retrieves it directly from Session.instance.

Pick one or the other, not both. If you do the former, keep in mind there’s the issue of methods like Track#starred, that can’t take the second parameter.

If libspotify ever gets support for multiple sessions, there’s room for a Session.with_instance(session) in the future. So I see no issues with the latter implementation.

Following error is raised when loading Hallon:

in `attach_function': Function 'sp_error_message' not found in [libspotify] (FFI::NotFoundError)

This is a result of name mangling in windows DLLs, and I’ve created an issue for the libspotify dudes: http://getsatisfaction.com/spotify/topics/libspotify_windows_dll_contain_mangled_function_names

As of currently, this can be hacked (really, it’s evil) around by hooking the attach_function method in FFI::Library, and then iterating over the arguments to calculate the total byte size to finally mangle the name before trying to bind it.

Somewhat related to issue #25.

THIS IS HORRIBLE :(

Also, look over the specs while you are at it. Session.instance expectations and expectations of arrays of Hallon objects with certain pointers would be nice to refactor, too.

Possible solutions:

1. Pass a Hash<Object,Event> (would allow one to listen for events on multiple objects):

   session.process_events_on(playlist_container => [:container_loaded], playlist => [:playlist_state_changed])

   Would be useful waiting for multiple tracks to load, for example (?). But… YAGNI?

2. Allow passing the object to listen to as the first parameter:

   session.process_events_on(playlist, :playlist_state_changed, :playlist_metadata_updated)

   This would only allow you to listen for events on a single object, and this is the simplest thing
   that could possibly work. I think I will go with this implementation first, and later refactor if needed.

Number 2 has the benefit of making it easier to implement #24, as well.

https://github.com/Burgestrand/Hallon/blob/master/ext/hallon.c#L127 <- this is bad.

One funny way to solve this would to put an event in the event queue that kills the event thread when executed. Of course, none of the libspotify callbacks after this will be handled.

See show_playlistcontainer_for_user for an example of where this"d be useful.

Some specs right now only test to see that the correct libspotify functions are called. Should we instead add accessors, so it can be verified through the C code instead?

… maybe!

Under the hood, it seems that libspotify might call callbacks as a side-effect of calling sp_link_create_from_string. If it does, the process will deadlock.

This issue is probably better fitting in Burgestrand/libspotify-ruby, but for now it’ll reside here. It would be nice with a list from the libspotify devs of which functions might exhibit this behaviour (that, or assume all are blocking, could be done I guess).

Right now we’re testing both Hallon, as well as libspotify itself. Considering we use FFI now, we could easily mock the API in Ruby, without having to write any C code for it.

Now, do we need to mock it? One could argue it’s sufficient to make sure the proper API methods are called, but then how do we handle the callbacks? Even worse, how do we know the libspotify bindings are correct (with types and such)?

Testing live against Spotify is fragile, requires an internet connection, a premium account and a great deal of faith from the person running the tests (making sure I don’t do anything evil on the account). However, it does make sure Hallon works as expected when using it for real.

As of now, live testing will do, but this is an open question.

• pyspotify is tested using a mock module
• libspotify-node tests live
• Hallon (as of current) tests live

As libspotify is asynchronous, Hallon also aims to provide asynchronous callbacks. However, in order to test Hallon properly, one needs to write the code in a slightly more synchronous fashion. Doing this in a clean way is essential, both for using the library and for testing it. It’s proven hard to solve, but there’s a clear pattern in how I use the library.

Making sure no events are fired in the meantime, we:

1. define event handlers on various objects (setting them up so we can wait for them to fire later)
2. call our Hallon API method (login, load playlist etc)
3. wait for callbacks from point no. 1 to fire, doing a specific action for each callback
4. once a user-defined condition has been fulfilled, program flow continues

I’ll keep this issue up until I have a robust solution. For now, I have Session#process_events_on which should suffice for simple cases.

Input:
require 'lib/hallon'
Hallon::Link.new 'spotify:track:4yJmwG2C1SDgcBbV50xI91'

Output:
[BUG] Segmentation fault

I believe this is an issue with libspotify.

Once the event is fired, the handlers are to be executed in the order they were added. Handlers can be removed, and the execution chain should be interruptible, allowing any handler to hide the event from the other handlers to be executed next.

Given a block, the returned Enumerator will have a reference to the “self” of the current track. Since this block is stored, Ruby needs to keep this reference for as long as the Enumerator exists.

What we should be doing is simply just keeping a reference to the @pointer. We don’t care about anything else.

1. trying to create a session but it fails, then releasing it (might be a bug, might be intended)
2. creating a session then releasing it (libspotify bug, happens about 5-10% of the time)

I will not use sp_session_release at all until the bug is fixed. It is in the code, but commented out.

Session#process_events_on now merely runs the block, without arguments, to check if proper conditions are met so one could continue.

However, if the name of the fired event, and its’ arguments, were given, one could more intelligently decide what should happen next.

I want flexibility, I want watchr

Will file YARD issue when I have time to write minimal example code.

Support for image loading would be greatly appreciated.

If I only understood C, I would be more than willing to help implementing this.

libmockspotify for unit tests, which is nice because it means the unit tests will be speedy and some-what accurate.

However, to approach proper test coverage one would want some more thorough (and slower) tests, which would also serve as usage examples. These would log in to the Spotify service, perform a series of tasks, assert that they’ve been executed properly, clean up and then exit.

Examples of this are:

• loading an image
• loading track information
• loading a playlist
  … etc

This’ll have to wait until after the 25th (payday).

… that is bad.

Suggestion: add a context for operations that require you to be offline / online

Hallon should have a 0.x.y-version already (because of the current features). Construct a simple versioning policy and add it to the README.

For some reason, requiring Resque and Hallon at the same time seem to cause things to come to a deadlock.

To reproduce, one should:

1. Create a brand new Rails 3.0.x application (might not be necessary with rails, but it’s what we had just now).
2. Add Hallon (HEAD) and Resque to the Gemfile.
3. Start the console.
4. Create a session.
5. Log in to Spotify.
6. Create a Link, a Track or whatever. Process events and stuff; just using Hallon causes it to hang.

Track.local calls Spotify.localtrack_create, which by name convention returns a track with its’ refcount increased. This means that in Track#initialize we should not be adding a ref to this pointer, but we do.

Now that I think of it, we could probably support Track.local by means of Track.new, simply by checking which arguments we’re given. If it’s one argument, we’re fetching an existing track; if it’s two, we’re creating a new local track. Bad idea? Not sure yet.

The UBF function should only interrupt the blocking function call. For example, unlocking the mutex or signaling the waiting condition. However, this cannot be reliably done for one thread so I need a special UBF for this.

If signals are delivered during these two pthread functions I believe RUBY_UBF_PROCESS uses SIGVTALARM. Maybe that can be used?

PS: This is only an issue if a thread needs to be killed while blocking on one of these methods. As it is now, it will not die. If the main thread deadlocks on a wait/lock (in a rb_blocking_region) the program will never exit.

Spotify also noted that all older libspotify versions are now deprecated. Yay cutting edge \o/

Swap session.c:116 with session.c:119. It will cause “rake spec” to segfault, as well as ruby -Ilib -Ispec -rsupport/config -rhallon -e 'require "bundler/setup"; exec("rspec spec/**/*_spec.rb")'

Don’t swap them and everything runs fine.

It’s proven to be a tricky bug for me to solve. For some reason, the condition being waited on in events.c:27 is missing out on signals from the libspotify callbacks. The event_mutex lock is held when events.c is processing any events; and when it is not it is waiting on the event_cond. How can this be!?

Related files:

• (brings C events into ruby) event producer: events.c

  Starts out by locking the event_mutex before anything else can be done. It
  then goes into a tight loop, waiting on signals on event_cond. When a signal
  is received, the shared memory structure is used to build an object in ruby
  equivalent to the C event.

• (saves C events in shared memory) callbacks.c

  In here is the code that actually locks the event_mutex, fills the event
  structure and signals the event producer. The idea is that the event_cond is
  being waited on by the event producer, and when signalled it will pick up the
  event structure and dispatch it within ruby.

A proper solution to this problem would hopefully solve #41 as well.

Currently, pretty much all Hallon objects accept an FFI::Pointer as an argument. Problem with this is that Hallon cannot know if this pointer needs its’ reference to be upped or not, but tries to make an educated guess. Most of the time this works, but it’s unreliable and frankly it makes the code harder to grasp.

In addition, Hallon also wraps this pointer into a Spotify::Pointer. Even at this point, we do not know if this pointer has an added reference. Still, this is the only object we care about existing in the first place.

Now, assuming we convert fully into using only AutoPointers (and such, Spotify::Pointer), we need some basic assumptions:

1. If we have a Spotify::Pointer, its’ GC is fully taken care of (and will be released when it’s GC’d by ruby).
2. If we don’t have a Spotify::Pointer, we can wrap an existing pointer and make it one.
3. Wrapping an existing pointer into a Spotify::Pointer means we extend the life of the underlying pointer until the Spotify::Pointer is released by ruby (at which point we free it).

This spawns the question, who is responsible for giving life to the underlying possible with add_ref, if necessary? Is it the creator, or the wrapper? Thinking of it for a while makes it pretty clear that the wrapper object itself should be self-managing, and if it’s given a pointer that already has its’ refcount at 1 instead of 0, it’s the callers’ duty to decrease the refcount by one after wrapping the pointer.

As a final crazy idea, I had this thought that I could monkeypatch/wrap/proxy the Spotify API, so that the pointers it returns always has their refcount set at 0. That would make this a non-issue for me, but it would make it harder to use the raw API in conjunction with Hallon, and I think it’s already hard enough, tyvm.

Top priority on this at the moment. Creating the issue so I remember my thought process tomorrow morning when I’m sane enough to judge my own ideas (and not subject poor Elin to them).

I made an issue for this because I’d like to tick it off once I reach it.

Currently at 92.00%.