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

iPod sync alpha release #104

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@jnwickremasinghe
Contributor

jnwickremasinghe commented Sep 21, 2013

Thomas,

Here's the alpha release for iPod sync. It works well apart from the fact that the UI locks up during the sync process. I can't figure out why this is happening. The sync process is running in a separate thread, and I'm calling the reporthook after each episode is copied, but the UI is still locking up on the call to gpod.itdb_cp_track_to_ipod. (If I put a breakpoint on the line after the copy, everything works fine and I can interact with the UI while the sync thread is suspended, and it'll stop at the breakpoint with a refreshed UI after each episode, but when I disable the breakpoint and continue the UI locks up.) The process does eventually finish though, and copies the episodes to the device ok, so it's more of an annoyance than a critical issue.

Anyway, maybe we can put it as an alpha feature (as you suggested) and get feedback from users.

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 21, 2013

The comment here should probably include 'ipod' now.

The comment here should probably include 'ipod' now.

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 21, 2013

Should we join this with 'device_folder', so that depending on the 'device_type' set, 'device_folder' works for both the filesystem-based player and the iPod?

Should we join this with 'device_folder', so that depending on the 'device_type' set, 'device_folder' works for both the filesystem-based player and the iPod?

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 21, 2013

Why did we convert this to a str again? Didn't it work without? If so, we might want to use one of the conversion functions from the gpodder.util module to convert from an unicode string to a UTF-8 encoded string.

Why did we convert this to a str again? Didn't it work without? If so, we might want to use one of the conversion functions from the gpodder.util module to convert from an unicode string to a UTF-8 encoded string.

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 21, 2013

Rewrite as:

podcasturls = [track.podcasturl for track in tracklist]

Rewrite as:

podcasturls = [track.podcasturl for track in tracklist]

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 21, 2013

Feel free to just remove these two lines instead of commenting them out.

Feel free to just remove these two lines instead of commenting them out.

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 21, 2013

Member

Review done. Please have a look at the comments and ask if anything is unclear or I made a mistake somewhere :) Other than that, I can see this patch going in. Thanks for keeping onto this, it's really appreciated!

Member

thp commented Sep 21, 2013

Review done. Please have a look at the comments and ask if anything is unclear or I made a mistake somewhere :) Other than that, I can see this patch going in. Thanks for keeping onto this, it's really appreciated!

@thp

This comment has been minimized.

Show comment
Hide comment
@thp

thp Sep 24, 2013

Member

After addressing the comments from above, I'd like to merge this into the Git master branch so the users on the bleeding edge can give this a try so that we might eventually be able to include it in the next public release.

Member

thp commented Sep 24, 2013

After addressing the comments from above, I'd like to merge this into the Git master branch so the users on the bleeding edge can give this a try so that we might eventually be able to include it in the next public release.

@jnwickremasinghe

This comment has been minimized.

Show comment
Hide comment
@jnwickremasinghe

jnwickremasinghe Nov 4, 2013

Contributor

Addressed in PR #108 which has been merged. This request can be closed.

Contributor

jnwickremasinghe commented Nov 4, 2013

Addressed in PR #108 which has been merged. This request can be closed.

@jnwickremasinghe jnwickremasinghe deleted the jnwickremasinghe:ipod-sync branch Nov 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment