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

[Binary Addon] Provide instance version when creating AddonInstance #15369

Merged
merged 1 commit into from Feb 6, 2019

Conversation

@peak3d
Copy link
Contributor

commented Jan 30, 2019

Description

[Binary Addon] Provide instance version when creating AddonInstance

This PR adds an optional CreateInstance overridable method which provides the interface version compiled inside kodi. No adaption of existing addons necessary.

Beside this the current addon system is not downgrade capable for binary addons.
If for example inputstream binary has API verMin 1.0.0 and ver 1.0.9, but kodi is compiled with API verMin 1.0.0 and ver 1.0.8, addon is treated as incompatible, which shouldn't because addons min version is smaller as kodi version. This is fixed by retieving the API min version from compiled addon.

Motivation and Context

For the binary addon inputstream.adaptive I need to know which API version is compiled in kodi to implement workarounds for the combination old(er) kodi + newer inputstream addon.

How Has This Been Tested?

inputstream addon, with / without this PR combinations.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)
@peak3d peak3d modified the milestone: Leia 18.1-rc1 Jan 30, 2019
@peak3d peak3d added v19 Matrix and removed v18 Leia labels Jan 30, 2019
@peak3d peak3d force-pushed the peak3d:addonversion branch 2 times, most recently from 999f03b to 0851307 Jan 30, 2019
@peak3d peak3d added this to the Leia 18.1-rc1 milestone Jan 30, 2019
@peak3d peak3d force-pushed the peak3d:addonversion branch from 0851307 to 5d2e842 Jan 31, 2019
Copy link
Member

left a comment

Looks good so far.

return kodi::addon::GetTypeMinVersion(type); \
} \
AddonGlobalInterface* kodi::addon::CAddonBase::m_interface = nullptr; \
std::string kodi::addon::CAddonBase::m_strGlobalApiVersion;

This comment has been minimized.

Copy link
@AlwinEsch

AlwinEsch Feb 1, 2019

Member

A new line is missing :)

@@ -320,8 +329,14 @@ class ATTRIBUTE_HIDDEN CAddonBase
return ADDON_STATUS_UNKNOWN;
}

virtual ADDON_STATUS CreateInstanceEx(int instanceType, std::string instanceID, KODI_HANDLE instance, KODI_HANDLE& addonInstance, const char *version)

This comment has been minimized.

Copy link
@AlwinEsch

AlwinEsch Feb 1, 2019

Member

You use this way that an addon can check itself to determine if it is compatible?

As far as OK, I would instead of "const char *" a "std :: string", this fits more to the C ++ interface and makes a string compare easier.

kodi::addon::GetTypeVersion(type),
kodiMinVersion.asString().c_str(),
addonMinVersion.asString().c_str(),
addonVersion.asString().c_str());

This comment has been minimized.

Copy link
@AlwinEsch

AlwinEsch Feb 1, 2019

Member

On newest CLog is no more a "c_str()" required and the %s if wished can be used mit {}.

#define RESOLVE_METHOD_RENAME_OPTIONAL(dllmethod, method) \
m_##method##_ptr = nullptr; \
m_dll->ResolveExport( #dllmethod , & m_##method##_ptr );

This comment has been minimized.

Copy link
@AlwinEsch

AlwinEsch Feb 1, 2019

Member

Really super good to have :D

@peak3d peak3d force-pushed the peak3d:addonversion branch from 5d2e842 to f83688a Feb 1, 2019
@peak3d

This comment has been minimized.

Copy link
Contributor Author

commented Feb 2, 2019

@AlwinEsch fine now?

@@ -320,8 +329,14 @@ class ATTRIBUTE_HIDDEN CAddonBase
return ADDON_STATUS_UNKNOWN;
}

virtual ADDON_STATUS CreateInstanceEx(int instanceType, std::string instanceID, KODI_HANDLE instance, KODI_HANDLE& addonInstance, const char *version)

This comment has been minimized.

Copy link
@AlwinEsch

AlwinEsch Feb 2, 2019

Member

Here you have still the "const char *" in IAddonInstance it is it std::string :)

@peak3d peak3d force-pushed the peak3d:addonversion branch from f83688a to 64a933b Feb 4, 2019
@peak3d

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2019

jenkins build this please

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

Any objections for adding this to point release?

@Rechi

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@peak3d you tested the following 4 combinations (same version obvious isn't that interesting?

  • (compiled addon without this PR, run Kodi without this PR)
  • (compiled addon with this PR, run Kodi with this PR)
  • compiled addon without this PR, run Kodi with this PR
  • compiled addon with this PR, run Kodi without this PR
@peak3d

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2019

Only the last 3 @Rechi

@MartijnKaijser MartijnKaijser merged commit 1711550 into xbmc:master Feb 6, 2019
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.