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

fix bug causing recordings not to display for older TVH #56

Merged
merged 1 commit into from Jul 3, 2015

Conversation

Projects
None yet
3 participants
@adamsutton
Contributor

adamsutton commented Jul 2, 2015

This was caused by a mistake in the HTSP spec documentation. Also
correctly some un-init vars that could have resulted in undefined
behaviour.

Adam Sutton
fix bug causing recordings not to display for older TVH
This was caused by a mistake in the HTSP spec documentation. Also
correctly some un-init vars that could have resulted in undefined
behaviour.

@Jalle19 Jalle19 added the Fix label Jul 3, 2015

Jalle19 added a commit that referenced this pull request Jul 3, 2015

Merge pull request #56 from adamsutton/master
fix bug causing recordings not to display for older TVH

@Jalle19 Jalle19 merged commit 1d1f409 into kodi-pvr:master Jul 3, 2015

@Jalle19

This comment has been minimized.

Contributor

Jalle19 commented Jul 3, 2015

Thanks for the fix, I'll handle the backporting.

@ksooo

This comment has been minimized.

Member

ksooo commented Jul 3, 2015

Just for your information. The retention handling fix is relevant for Isengard only, as we decided to drop support for ancient tvh versions with J******. The many runtime protocol checks needed for this, esp. for autorec, timerec, ... are cluttering the code to much.

@adamsutton

This comment has been minimized.

Contributor

adamsutton commented Jul 3, 2015

@ksooo personally I think that's a very bad idea. I'm not entirely happy with how new features, which are clearly "optional" (like retention) having been listed as "required" in the HTSP spec. Something I will have to take up with @perexg should I ever get back onto TVH work.

Calling my version of TVH ancient is very much over stating the fact. I admit that I'm quite a bit behind master these days, but I'm still using a non-stable version from when I was running the project which was considerably a head of the "stable" versions of the time and so at most should really only be considered a year or so old. Maybe by some modern standards that's "ancient", but by most sane standards it most certainly isn't.

What my version is is STABLE. It's been running now for 5 months, and before that about the same again (only down because of a power failure). It does what it needs to, and simply breaking the integration between that version and Kodi just "because" would be a very bad idea!

And more so most of the "extra" HTSP protocol version checks that have been added since I wrote this code are completely redundant. It should be fairly obvious that by definition something added in a future version of HTSP is pretty much by definition "optional", since the previous versions managed quite happily without them.

The only stuff where the version check is required is where for one reason or another, and I was quite careful to limit this, the API changed in a non-backward compatible manner. I.e. the content of some previously existing field actually changed in meaning (there are about 2-3 instances of this).

If you want to drop support for "ancient" versions of HTSP, then really at most you want to be dropping support for anything prior to about v12, which I think was done a long time ago.

OK, rant over!

@Jalle19 Jalle19 referenced this pull request Jul 4, 2015

Merged

backport of #56 to Isengard #58

Jalle19 added a commit that referenced this pull request Jul 4, 2015

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