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 addon install if not present before #12825

Merged
merged 3 commits into from
Sep 26, 2017

Conversation

AlwinEsch
Copy link
Member

@AlwinEsch AlwinEsch commented Sep 23, 2017

On last changes comes the problem that a addon install who was not present
before has not worked.

This comes from a try to disable a addon who is not present and brought a
false.

With them becomes before checked that a addon is present, otherwise is this
step ignored.

Related to #12473 (review)

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@AlwinEsch AlwinEsch added Component: Add-ons Type: Fix non-breaking change which fixes an issue labels Sep 23, 2017
@AlwinEsch AlwinEsch added this to the L 18.0-alpha1 milestone Sep 23, 2017
@AlwinEsch
Copy link
Member Author

@tamland is this fix OK for you, that we bring it in asap?

@tamland
Copy link
Member

tamland commented Sep 24, 2017

This check needs to be move inside UnloadAddon (I.e it should return true if the addon isn't installed). Otherwise there are race conditions

@AlwinEsch
Copy link
Member Author

Is changed.

Think to replace the CAddonMgr::GetInstance() with CServiceBroker::GetAddonMgr() on all places on another request

@stefansaraev
Copy link
Contributor

works for me. thanks @AlwinEsch @tamland

@AlwinEsch
Copy link
Member Author

Install test has worked, but there seems still a fault with start of a new installed addon.

If I have installed YouTube and start it are the strings from them not visible. Also brings it a Python error after select of a field.

If Kodi is restarted does it work, there seems the loading of new installed addon not work!

@stefansaraev
Copy link
Contributor

yep same behaviour here. I thought it was unrelated but indeed completely reverting #12473 makes those strings work again...

On last changes comes the problem that a addon install who was not present
before has not worked.

This comes from a try to disable a addon who is not present and brought a
false.

With them becomes before checked that a addon is present, otherwise is this
step ignored.
@AlwinEsch
Copy link
Member Author

Have added a second commit who fix the fault with not usable addon after install.

The fault itself was coming after the change of the function value AddonPtr& addon to const std::string& addonId on the LoadAddon (old ReloadAddon).

If all commits are reverted and only this with change to std::string is done, comes the fault.

The function has changed the addon class on CAddonInstaller.

@@ -614,12 +614,15 @@ bool CAddonInstallJob::DoWork()
if (!Install(installFrom, m_repo))
return false;

if (!CAddonMgr::GetInstance().LoadAddon(m_addon->ID()))
AddonPtr addon = CAddonMgr::GetInstance().LoadAddon(m_addon->ID());

This comment was marked as spam.

This comment was marked as spam.

@AlwinEsch
Copy link
Member Author

Is changed to use GetAddon

Copy link
Member

@tamland tamland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Merge when ready

@AlwinEsch AlwinEsch merged commit 6f99b6e into xbmc:master Sep 26, 2017
@Rechi Rechi added the v18 Leia label Sep 26, 2017
@AlwinEsch AlwinEsch deleted the fix-addon-change branch March 20, 2018 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Add-ons Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants