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

[PVR] PVR windows: Fix channelgroup listener registration. #11305

Merged
merged 1 commit into from Dec 31, 2016

Conversation

@ksooo
Copy link
Member

commented Dec 30, 2016

This fixes a regression that slipped in due to the pvr window listener logic changes I did some months ago.

Visual effects of the issue are for instance an empty channel window after changing kodi's ui language and no live update of the channel or guide window in case channels get added or removed in the backend.

Fix has been runtime tested on Linux and macOS on latest Kodi master.

@Jalle19 mind doing the review.

@ksooo ksooo added this to the L 18.0-alpha1 milestone Dec 30, 2016
@ksooo ksooo requested a review from Jalle19 Dec 30, 2016
@@ -84,7 +84,7 @@ namespace PVR

bool InitChannelGroup(void);
virtual CPVRChannelGroupPtr GetChannelGroup(void);
virtual void SetChannelGroup(const CPVRChannelGroupPtr &group);
void SetChannelGroup(const CPVRChannelGroupPtr &group, bool bUpdate = true);

This comment has been minimized.

Copy link
@Jalle19

Jalle19 Dec 30, 2016

Member

Do you mind adding a docblock so the purpose of the new boolean becomes clearer?

This comment has been minimized.

Copy link
@ksooo

ksooo Dec 30, 2016

Author Member

Sure, can do. Sigh, I wish my predecessors would have been more professional on this. But ofc this is no excuse for not adding docs for new stuff.

This comment has been minimized.

Copy link
@ksooo

ksooo Dec 30, 2016

Author Member

doxy added.

@@ -300,9 +300,10 @@ bool CGUIWindowPVRBase::InitChannelGroup()
if (m_channelGroup != group)
{
m_viewControl.SetSelectedItem(0);
m_channelGroup = group;
m_vecItems->SetPath(GetDirectoryPath());
SetChannelGroup(group, false);

This comment has been minimized.

Copy link
@Jalle19

Jalle19 Dec 30, 2016

Member

Can't see the full context on the phone so are you sure that both this method and SetChannelGroup() don't attempt to grab the same lock?

This comment has been minimized.

Copy link
@ksooo

ksooo Dec 30, 2016

Author Member

that both this method and SetChannelGroup() don't attempt to grab the same lock

they do, but whats the problem with this? performance? no. these methods are not getting called very often.

@ksooo ksooo force-pushed the ksooo:pvr-fix-windows-channelgroup-listeners branch from 5428dae to d18a2cc Dec 30, 2016
@ksooo

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2016

@Jalle19 good to go now?

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2016

jenkins build this please

@ksooo

This comment has been minimized.

Copy link
Member Author

commented Dec 31, 2016

jenkins errors are not related to this PR.

@ksooo ksooo merged commit f8d7ed1 into xbmc:master Dec 31, 2016
2 of 3 checks passed
2 of 3 checks passed
jenkins4kodi You are a failure. Fix the code and try again......
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ksooo ksooo deleted the ksooo:pvr-fix-windows-channelgroup-listeners branch Dec 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.