-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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] ability to sort channels and epg by the backend order of channels #16558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, did not went through the whole changes. -1 for two reasons:
-
implementation breaks PVR object call hierarchy. CPVRChannel and CPVREpgInfoTag must not depend on any of the channel containers.
-
API needs to be extended to transport the order. One should not rely on creation order.
If you need to transfer any channel information to CPVREPGInfoTag, do this via its CPVREpgChannelData member. If you need the order at the CPVRChannel, introduce a member m_iOrder (or the like) at CPVRChannel. |
Ok, for point 2) I know what to do. However for point 1) it’s a bit trickier as both Channel and EpgInfoTag would need to know the current group member that they pertain to without breaking the design. I’ll have to think on that. |
8ff89a7
to
7387380
Compare
@ksooo, should now satisfy both rejection points. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally okay now. Needs a bit more love, though.
Jenkins build this please |
7387380
to
4daf89c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let the nitpicking begin...
4daf89c
to
cdcba3c
Compare
Commit message seems unrelated to this PR. ;-) |
cdcba3c
to
5c36eac
Compare
Ya, ran the wrong history command, fixed now. |
So, does it still work after all the review induced changes? |
Yes, why do you think the changes would effect it working? Did you spot something? |
No. I was just asking. |
Cool, tested with iptvsimple and with also with vuplus. Works with both. |
From a user's point of view, what would you expect to happen, if you select "Backend order" if the pvr client addon does not fill the new iOrigin field? Nothing? Hmmm... Shouldn't we add a fallback logic to sort by backend channel numbers, then? |
Yes, that makes sense, agreed. That's the missing piece. |
It might be simpler in UpdateClientOrder to set it to the backend channel number if iOrder is 0. EDIT: this won't work as ChannelNumbers are different. |
5c36eac
to
8aa2864
Compare
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost done, I'd say.
8aa2864
to
2879e30
Compare
Cool, thx. |
Ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we go. :-)
@ksooo please always set milestone before or right after merging a PR. |
Woohoo! |
Done. |
Nice work ;) At some point I'll write a script to automate this process, might make it a little less painful. |
That's definitely a good idea. |
Description
Add the ability to sort channels and epg by the backend order of channels. Instead of extending the API I just used the creation order for the value or sort by.
Resolves issue: #16546
Motivation and Context
Allows the preservation of order of the backend pvr if that system does not use channel number based ordering.
How Has This Been Tested?
Using pvr.vuplus and pvr.iptvsimple.
Screenshots (if appropriate):
Types of change
Checklist: