/web/epgbouquet does not behave the same as in Version 1 API - causing issues.
Closed this issue · 37 comments
This bug occurs in both the Modern and Classic web interfaces.
A call to:
/web/epgbouquet?bref=<>
returns the entire EPG for that bouquet.
In version 1 it would only return the EPG contents at a single point in time (teh optional time parameter being when you want this to be, and default is "now). So there is no endTime parameter either.
/web/epgbouquet now behaves exactly the same as /web/epgmulti.
This is causing a problem for things such as the dreamDroid Android app, which is using this call to get the service entries in bouquet. Instead of getting a quick repsonse it is taking >30s to get a >7MB response for a bouquet with 72 services in it.
I would expect this to replicate how the function behaved in the version 1 code (at https://github.com/E2OpenPlugins/e2openplugin-OpenWebif).
At the same time "dummy" services in the bouquet (i.e. those added to get the required service number) are now being returned with an e2eventservicename of "<n/a>" (<n/a>) rather than being ignored and having no entry returned at all.
Has this issue started to happen just recently?
Yes. It happened when OpenVix switched to using the version 2 code.
/web/epgbouquet and /web/epgmulti is 100% the same.
Someone has created this issue in version 1.x .. and I have taken this to 2.0.
It is different in version 1 (or was just a few weeks back)
To return the correct timespan just set endTime to beginTime (there is no endTime parameter in version 1)
But also need to remove the n/a data in the output.
The API documentation is quite specific as to what each should do:
https://github.com/E2OpenPlugins/e2openplugin-OpenWebif/wiki/OpenWebif-API-documentation#epgbouquet
I think this commit is the problem:
Yes. But its a bug. Better not to copy bugs....
Please try
endtime = None
in line 1444
I think this issue is not new because the involved code has no changes last year.
endtime = -1 or endtime = None has no effect.
Are you sure that the current version 1.x is correct?
I doubt current 1.x is correct but only just been noticed.
1.x has the same bug
Setting endTime to 0 works.. Just tested it. (-1 is what is there, and that fails).
(In the context endTime is actually an extent beyond beginTime. -1 means "get everything"...)
Can't figure out where the "n/a" is being generated - these entries should just be skipped.
The "n/a" is being added by lookupEvent() in epgcache.cpp.
I can get this removed by checking for service type in getBouquetEpg() (in models/services.py) before appending to the query and removing non-relevant types.
for service in services.getContent('S'):
stype = service.split(":", 3)
if stype[1] == "64": continue # Bouquet header
if stype[1] == "832": continue # Dummy number-padding entry
query.append((service, 0, begintime, endtime))
There may be others ("320"?).
With this, and the endTime fix, all works as it used to.
This is an api change or has someone removed this filter?
The "n/a" is being added by
lookupEvent()inepgcache.cpp.I can get this removed by checking for service type in
getBouquetEpg()(inmodels/services.py) before appending to the query and removing non-relevant types.for service in services.getContent('S'): stype = service.split(":", 3) if stype[1] == "64": continue # Bouquet header if stype[1] == "832": continue # Dummy number-padding entry query.append((service, 0, begintime, endtime))There may be others ("320"?). With this, and the
endTimefix, all works as it used to.
This is a wrong approach.
These are flags.
eServiceReference.isMarker=64
832 is just 64 with other flags. Look:
>>>
>>> 832 & 64
64
>>>
Same with 320...
>>> 320 & 64
64
>>>
So you need to import eServiceReference and check against the respective flags. e.g.
if int(stype[1]) & (eServiceReference.isDirectory | eServiceReference.isMarker | eServiceReference.isGroup): continue
And because you are now using the correct flags with explanatory names you don't need to add "helpful" comments.
I have no intention to do that, because this is an api change and may break some 3rd party tools.
Except someone has removed this filter.
I have checked version 0.x and there is no filter.
because this is an api change
It is not an API change.
It is reverting to the original (and documented - see above) API.
The code you have implemented is already breaking things!
Don't know why there were no n/as previously, but they've only started showing up recently and they should not be there.
So you need to import eServiceReference and check against the respective flags. e.g.
if int(stype[1]) & (eServiceReference.isDirectory | eServiceReference.isMarker | eServiceReference.isGroup): continueAnd because you are now using the correct flags with explanatory names you don't need to add "helpful" comments.
Whereas I agree that checking the bit pattern makes sense, that code cannot be correct and 320 (== 0x0140) only has 2-bits set, so must fail a test against any 3-bits being set, but it needs to pass the test.
EDIT: No, scrub that sentence. I was thinking about something else when I got to that code and ended up conflating the two to end up with a wrong conclusion.
Yes, that bit checker is fine.
Adding comments as to why you are skipping things can still be useful. I can read and understand the comment in-place, but have no idea what the meaning of isMarker or isGroup is.
PS: Github says I'm receiving notifications as I authored the thread - but I'm not actually getting any....
I (think) I can see where the "n/a" crept in.
The V2 code uses:
epgEvents = epg.getBouquetENowNextEpg(bqRef, begintime, endtime)
to get the events, whereas the V1 code does not.
Or at least it does not do so now. It did sort-of - it actually called getBouquetEevents) until Sep 9, 2022 when this revert:
E2OpenPlugins/e2openplugin-OpenWebif@b9e90b1
(by jbleyel) sent it back to what it had been, which was a self-contained loop.
So there was a filter there as that loop only added real events to the result.
Yes . V1 and V2 is different.
Let me check why lookupEvents is not working as expected
You seem to continue to ignore the fact that the epgbouquet and epgmulti codes are the same, when they are meant to be different.
epgbouquet should have no endTime parameter. This is documented. It is the equivalent of setting endTime to 0 (not to -1 as that means "get everything in the cache").
The extra parameter endTime has no effect until you don't set it.
So what's the problem?
The documentation is old and needs to maintain.
The extra parameter endTime has no effect until you don't set it.
And existing code does not set it.
So what's the problem?
The problem is that existing code does not set it and expects it to do what it is documented to do, and what it had been doing for years until recently.
The documentation is old and needs to maintain.
But the documentation is correct and reflects how the API has been used historically.
small update
oe-alliance/OpenWebif/wiki/OpenWebif-API-documentation#epgbouquet
That is totally useless!!!!!
All that does is make epgbouquet the same as epgmulti.
Stop treating the documentation as wrong. It is the current CODE which is wrong. It is already causing problems in long-existjng software!
The endTime parameter has to be removed and set to 0 (nothing else will work) internally.
Why is this marked as closed???
It's still causing problems for existing software!!!!
epgbouquet is, effectively, epqmulti with an endTime set to 0.
It could be implemented by such a call.
If you insist on giving epgbouquet an endTime parameter (which it never did) then set the default to 0 (just get "now") with epgmulti setting the default to -1 (get everything that is in the cache).
I‘m confused.
The endTime parameter is new and optional.
There is no endTime parameter in the old documentation.
The default is None and not -1.
I get only now events if i don’t use the parameter.
And this is exactly what we had before.
I‘m confused.
Clearly
The endTime parameter is new and optional.
But why? epgmulti was the call to make using an endTime parameter.
There is no endTime parameter in the old documentation.
Correct. And there was no endTime parameter in the call in the code either. It is the addition of this which is causing the problem with the amount of the data being returned.
(The "n/a" inclusion is because the new search code differs from the old).
The default is None and not -1.
The default should be 0, meaning a zero time extent. Somewhat clearer than None.
I get only now events if i don’t use the parameter. And this is exactly what we had before.
It is not what was happening when I opening this Bug Issue. You seemed to be concentrating on reproducing code that was already bugged, and ignoring documentation which was clear as to how the API should work.
You were talking about the endTime parameter when there is no need for this call to have such a parameter (it's meant to get the EPG at a single point in time) and it has never has such a parameter in the past. So if anyone should be confused it's me.
It's also unclear as to what has happened to the (erroneous) "n/a" entries.
Have you tested my last commit?
Probably not.
If the last change is also wrong then give me more info how to reproduce the issue.
Since you continue to mention that epgbouquet needs an endTime parameter (which it doesn't) I wasn't aware that you'd made any change in that area.
I'll try the latest epg.py, services.py and web;py to see what happens.
OK.
You're right.
Putting the latest epg.py, services.py and web.py in place makes it run as it used to do.
But I can still see no reason why you would wish to change the API by adding an endTime parameter to epgbouquet when epgmulti already provides that interface. That is just confusing.