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

[addons] fix event handling #10156

Merged
merged 4 commits into from Jul 31, 2016

Conversation

@tamland
Copy link
Member

commented Jul 23, 2016

Should hopefully fix all the various updating issues stemming from the lack of a event strategy in the addon system.
This includes a very basic pub/sub utility for sending events; it gets the job done but if anyone know something better, existing, that can be used, free to suggest one.

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 26, 2016

can someone provide xcode/vs for this?

@@ -253,6 +260,19 @@ void CDirectoryProvider::Fetch(std::vector<CGUIListItemPtr> &items) const
}
}

void CDirectoryProvider::OnEvent(const ADDON::AddonEvent& event)
{
CSingleLock lock(m_section);

This comment has been minimized.

Copy link
@wsnipex

wsnipex Jul 27, 2016

Member

couldn't the lock be moved down inside the if?

This comment has been minimized.

Copy link
@tamland

tamland Jul 27, 2016

Author Member

m_currentUrl needs to be protected as well, since it may be written to

@ksooo

This comment has been minimized.

Copy link
Member

commented Jul 28, 2016

@tamland you can cherry-pick the xcode project update for this PR here ksooo@1315dc0 if you like.

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 28, 2016

thanks @ksooo!

@tamland tamland force-pushed the tamland:addon_events branch from c2758f3 to 00353e7 Jul 28, 2016
@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 29, 2016

jenkins build this please

tamland added 3 commits Jul 23, 2016
the addon manager should be responsibile for reload and emitting events
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 29, 2016

Did quick test and seems to work fine now with installing Addons and updating vfs

@da-anda

This comment has been minimized.

Copy link
Member

commented Jul 30, 2016

sorry, haven't checked the changes - will this also update the add-on lists in terms of "recently used" on home screen? IIRC this is also not working atm (at least last time I checked).
Also, at least for me PVR channels and file shares in "videos" node not always show up, but that's a different issue.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 30, 2016

@da-anda yep for add-on question :)

@tamland tamland force-pushed the tamland:addon_events branch from 00353e7 to 8388d31 Jul 31, 2016
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jul 31, 2016

Can we include this for alpha3?

@tamland

This comment has been minimized.

Copy link
Member Author

commented Jul 31, 2016

yes. just had to squash.

@tamland tamland merged commit 772a7cf into xbmc:master Jul 31, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tamland tamland deleted the tamland:addon_events branch Jul 31, 2016
@MartijnKaijser MartijnKaijser added this to the Krypton 17.0-alpha3 milestone Jul 31, 2016
@BigNoid

This comment has been minimized.

Copy link
Member

commented Aug 1, 2016

This PR causes some issues. Easiest to reproduce is starting a video and do a ReloadSkin(). Without this PR it works fine, with this PR it crashes.

EDIT: see this post for a crashlog

@FernetMenta

This comment has been minimized.

Copy link
Member

commented on xbmc/utils/EventStream.h in 2702a31 Jan 12, 2018

events are not delivered in the same order as published. This is not what someone would expect from a class named CEventSource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.