Issues with Channel selection GREEN Add to favourite/Remove entry

[prl]

As well as the issues with using GREEN Remove entry to remove the current channel in the Favourites list, I'm not a big fan of the fact that GREEN Remove entry doesn't have the same "Are you sure you want to remove this entry?" popup as MENU>Remove entry, and I have some other issues with it and with GREEN Add to favourites.

• There is no visible user feedback about what was done on the GREEN Add to favourites action. The user can't tell whether the button press was acted on except by looking at the contents of the Favourites bouquet.
• It's much easier to invoke GREEN Remove entry accidentally than MENU>Remove entry (or MENU, 9, which has the same effect), and you may not even notice that you've done it until sometime later you wonder why a channel is missing from your Favourites list.
• The GREEN button has different actions in different bouquets (Add to favourites or Remove entry).
• The GREEN button's action in the list-of-bouquets screen is Add to favourites, and it adds the whole bouquet folder to Favourites. I'm not sure that's the intended behaviour.
• It's even possible to highlight Favourites, and then press Add to favourites. I've had both a UI hang (spinner forever) and a crash caused by doing this. The hang happened when the Favourites list was in its normal state and the crash happened when the Favourites list had a terrestrial TV LCN folder in it. After the crash, it became impossible to use the Favourites list to change channels, even though the Favourites bouquet file looked normal.

The GREEN Add to favourite/Remove entry feature looks like it needs more work.

[peteru]

Sounds like there's a whole bunch of issues there that will require several pull requests with a few commits in each. If you do decide to attack this, can you please split the work in such a way that there are several smallish pull requests, say one for fixing crashes/hangs, another for adding entries and yet another for removing? That will make it easier than having one pull request that covers all of the above.

It looks like I'll be completely flat out for the foreseeable future, so the easier it is for me to review and accept changes, the better.

In terms of progression, I'd suggest fixing bugs first, without addressing the UI shortcomings, then look at incremental improvements. For example, reassign buttons as one pull request and then implement a confirmation as another. You'll probably get better user feedback if you deal with one thing at a time too. Now that you presented the big picture for context, any detailed discussion for a particular feature can be framed in that context.

[peteru]

Speaking of enhancements and bug fixes:

* There's a long standing bug related to bouquet selection. If you do an initial setup and the scan fails to find anything, you end up with a weird state at the end of the wizard. The current bouquet resembles a partial database query.

* After rescanning, the users is left in the "Last Scanned" bouquet. This is not intuitive at all and generates unnecessary requests for help. It would be much better if the current bouquet was not changed at all. If it needs to be changed (for example because it becomes empty after the scan), then perhaps the user can be left in Terrestrial LCN or even All Services. However, my preference would be to not change the bouquet at all.

* Also, after a (re)scan, it may be best to not start any service so that there is no question as to what service to choose. Perhaps just present the user with a user interface that gives them a list of services and makes them pick one.

[dRdoS7]

Hi,

I'm not a big fan of the fact that GREEN Remove entry doesn't have the same "Are you sure you want to remove this entry?" popup as MENU>Remove entry.

Please don't add a popup for confirmation, unless there is an option to not ask. The whole point was to remove the many button presses required to add/remove channels.

dRdoS7

[MrQuade]

Please don't add a popup for confirmation, unless there is an option to not ask. The whole point was to remove the many button presses required to add/remove channels.

Haha, I was almost going to comment that this change would not be welcomed by dRdoS7!
If I recall this was an adoxa patch added to your requirements since you jigger with your bouquets so much.

Though surely if it was just a popup that just required you to press GREEN a second time, that wouldn't slow you down too much?

[prl]

Speaking of enhancements and bug fixes:

* There's a long standing bug related to bouquet selection. If you do an initial setup and the scan fails to find anything, you end up with a weird state at the end of the wizard. The current bouquet resembles a partial database query.

I haven't encountered that one, but doesn't sound nice.

* After rescanning, the users is left in the "Last Scanned" bouquet. This is not intuitive at all and generates unnecessary requests for help. It would be much better if the current bouquet was not changed at all. If it needs to be changed (for example because it becomes empty after the scan), then perhaps the user can be left in Terrestrial LCN or even All Services. However, my preference would be to not change the bouquet at all.

It would also be good to start a channel playing after a scan. After a scan from the Setup screen, perhaps the one that the user was last on, if possible.

* Also, after a (re)scan, it may be best to not start any service so that there is no question as to what service to choose. Perhaps just present the user with a user interface that gives them a list of services and makes them pick one.

Actually, the ServiceScan screen already has that interface, it just doesn't have any effect, but it could start the service that's currently highlighted when you press OK. There's a complication of what to do if that service happens to be a radio service, but perhaps that choice should be respected, too.

Another thing I meant to raise in pre-release, a bit trivial, but when you so a scan, in the scan status box the text for the network type is correct ("Terrestrial") but the corresponding icon is the cable icon (icons/scan-c.png).

[prl]

Please don't add a popup for confirmation, unless there is an option to not ask. The whole point was to remove the many button presses required to add/remove channels.

Haha, I was almost going to comment that this change would not be welcomed by dRdoS7!
If I recall this was an adoxa patch added to your requirements since you jigger with your bouquets so much.

Though surely if it was just a popup that just required you to press GREEN a second time, that wouldn't slow you down too much?

Well, it just happens that that's exactly what GREEN does in a MessageBox that doesn't ask a question.

I'd be OK with a user option not to notify, at least for add, but I think that there should always be a notification for remove, because it's a pain to undo one that's been done accidentally, perhaps without even being noticed. Note that because "yes" is the default option for the current "remove" popup, GREEN, GREEN would also delete when there was a popup for remove. Mind you, IMO "no" should be the default for the remove popup.

In any case, the cause of the apparent race condition that I've detailed in the linked post needs to be looked at in case it can also be triggered by someone who does the GREEN, GREEN very quickly.

[Spot]

[...]
In any case, the cause of the apparent race condition that I've detailed in the linked post needs to be looked at in case it can also be triggered by someone who does the GREEN, GREEN very quickly.

"... GREEN, GREEN very quickly".

Yes, I was in rapid-fire mode on pressing the green button to delete my favourites.

[prl]

That's not the scenario I was talking about.

I was talking about hypothetical actions where GREEN Remove entry opened a confirmation popup and that was quickly quickly dismissed by a second GREEN.

[MrQuade]

I'm guessing it would be a ton of work to introduce some sort of multi-select function like the one that was added to the media player?

[prl]

Apart from dRdoS7, how often do people build favourites lists? And anyway ,for anyone with a net-connected Beyonwiz, it's probably easier to do these sorts of bouquet manipulation using Open Webif.

[Grumpy_Geoff]

I'm guessing it would be a ton of work to introduce some sort of multi-select function like the one that was added to the media player?

Multi-select in the OWIF bouquet editor and hit delete

[prl]

Also, I think that if there was to be a "delete selected" function, the current bugs need to be sorted first.

[MrQuade]

Also, I think that if there was to be a "delete selected" function, the current bugs need to be sorted first.

Absolutely agree.

[prl]

Another thing I meant to raise in pre-release, a bit trivial, but when you so a scan, in the scan status box the text for the network type is correct ("Terrestrial") but the corresponding icon is the cable icon (icons/scan-c.png).

I've just confirmed that this problem only affects the V2.

The cause is that the data that decides which icon to show in the scan ultimately comes from eDVBFrontendData::getTypeDescription(), which returns the first network type in the list DVB-C, DVB-S, DVB-T, ATSC that the frontend supports. The V2's internal tuners support DVB-C and DVB-T, so it returns DVB-C, and that's the icon that's displayed.

The text for the network type is taken from eDVBTransponderData::getSystem() which returns the network type that the front end is actually set up to use.

There's no access to eDVBTransponderData for the scan from any Components.Source, so it can't be fixed just with skin changes.

It looks like the simplest fix would be to add a MultiPixmap that's updated from the same code that sets the text version of the network type.

That's a bit messy, because if the skin doesn't have a renderer element for ServiceScan["FrontendInfo"], GUISkin.createGUIScreen() will complain in the debug log, but if ServiceScan["FrontendInfo"] is removed, it isn't backward-compatible.

I still hate how Sucuri interferes with any technical discussions.

[peteru]

I still hate how Sucuri interferes with any technical discussions.

Jai. Pester him!

[sub3R]

... Mind you, IMO "no" should be the default for the remove popup. ...

I agree.

IMO this also applies to some other pop-ups that I’ve mentioned in the past, but I won’t go there.

[adoxa]

As well as the issues with using GREEN Remove entry to remove the current channel in the Favourites list [...]

Fixed.

There is no visible user feedback about what was done on the GREEN Add to favourites action. The user can't tell whether the button press was acted on except by looking at the contents of the Favourites bouquet.

When a service has been added the GREEN label now changes to "ADDED!"; when a service is already in Favourites, GREEN is blank.

It's much easier to invoke GREEN Remove entry accidentally than MENU>Remove entry (or MENU, 9, which has the same effect), and you may not even notice that you've done it until sometime later you wonder why a channel is missing from your Favourites list.

GREEN to remove now uses the same dialog as the menu (and I disagree that "No" should be default). (I thought "session" meant while it was open, but it means until restart. I did try restoring confirmation on exec begin, but that breaks the menu.)

The GREEN button has different actions in different bouquets (Add to favourites or Remove entry).

Deliberate design decision, not changing.

The GREEN button's action in the list-of-bouquets screen is Add to favourites, and it adds the whole bouquet folder to Favourites. I'm not sure that's the intended behaviour.

Fixed - directories are always ignored by GREEN.


Now to go back and split it up for submission...

[prl]

The GREEN button has different actions in different bouquets (Add to favourites or Remove entry).

Deliberate design decision, not changing.

I don't have a problem with that per se. It was that in combination with the fact that there was little visual feedback and the fact that it was IMO too easy to make an accidental deletion that was the context that I though made it problematic. I though that would have been clear from the context.

Now that the other issues have been addressed, I have no problem with the change in the button's function.

I'm still of the opinion that it should be a "no" default on Remove entry.

[MrQuade]

When a service has been added the GREEN label now changes to "ADDED!"; when a service is already in Favourites, GREEN is blank.

How long does it stay displaying "ADDED!"? I presume it stays as "ADDED!" until you navigate away, and then when you navigate back to that entry it will be blank?

I would have thought it would be sufficient to just blank the button as soon as the service is added to favourites rather than having this intermediate "ADDED!" state.

[adoxa]

How long does it stay displaying "ADDED!"? I presume it stays as "ADDED!" until you navigate away, and then when you navigate back to that entry it will be blank?

That's right.

I would have thought it would be sufficient to just blank the button as soon as the service is added to favourites rather than having this intermediate "ADDED!" state.

It crossed my mind, but ended up as it is; I'll change it.

[prl]

I would have thought it would be sufficient to just blank the button as soon as the service is added to favourites rather than having this intermediate "ADDED!" state.

It crossed my mind, but ended up as it is; I'll change it.

I'm not sure exactly what;s intended here, but I'd prefer it if Add to favourites was only shown on the button if the Favourites bouquet doesn't already contain the service (and would, of course, then go blank if the current entry is added to Favourites).

"Add to favourites" will do nothing if the service is already in Favourites , anyway.

[MrQuade]

I'm not sure exactly what;s intended here, but I'd prefer it if Add to favourites was only shown on the button if the Favourites bouquet doesn't already contain the service (and would, of course, then go blank if the current entry is added to Favourites).

"Add to favourites" will do nothing if the service is already in Favourites , anyway.

Yes, that's what it is currently doing.

[prl]

* There's a long standing bug related to bouquet selection. If you do an initial setup and the scan fails to find anything, you end up with a weird state at the end of the wizard. The current bouquet resembles a partial database query.

Could you be more specific about the starting point and what the "weird state at the end of the wizard" is?

I did a Factory reset on a V2 with the antenna disconnected, and it ran pretty much as expected. At the end when it showed the Terrestrial LCN bouquet, that screen couldn't be dismissed by OK, because there were no channels to select. It had to be exited with EXIT.

The bouquets (and their files) were all empty apart from their #NAME entries. There was no userbouquet.terrestrial_lcn.radio bouquet created (and no reference to it in bouquets.radio), and the #NAME entry in userbouquet.terrestrial_lcn.tv was Terrestrial LCN rather than Terrestrial TV LCN.

Today's Sucuri whinge relates to "#NAME".

[prl]

I'm not sure exactly what;s intended here, but I'd prefer it if Add to favourites was only shown on the button if the Favourites bouquet doesn't already contain the service (and would, of course, then go blank if the current entry is added to Favourites).

"Add to favourites" will do nothing if the service is already in Favourites , anyway.

Yes, that's what it is currently doing.

Yes, the fact that you can't add the same service twice to a bouquet is already in the code and predates adoxa's changes.

What I'm saying is that the GREEN button Should only show "Add to favourites" (in non-Favourites bouquets) if pressing GREEN will actually add the service to Favourites, that is, the service isn't already in Favourites.

If that's implemented consistently, that's all that is needed to blank "Add to favourites" from the GREEN button when a service is added to Favourites. That is not implemented in the current code.

Doing it that way provides more information than changing the button label to "ADDED" or blanking the button name when a service is added (in that suggestion, what would the GREEN button hint show for GREEN, DOWN, UP?).

The construction of the "Choose bouquet" menu in MENU>Add service to bouquet should also be smarter and only show the bouquets that the service can be added to: i.e. those bouquets that don't already include that service. Doing that would also exclude the bouquet that is the source of the "add service" from the list, since by definition, it includes that service.

[peteru]

Sorry, at this stage I don't have access to a system that I could test with, so I can't be more specific. Now that I think about it a bit more, it may have been caused by exiting the wizard early, which can be done with the exit key (possibly used multiple times to exit the sub-wizards).

From memory, the issue may be reproducible if you stop enigma2 and clear/delete some of the config files in /etc/enigma2, then start it up again.

Sorry, for such vague info... I feel guilty, but I don't have the means to provide anything specific right now.

[MrQuade]

Yes, the fact that you can't add the same service twice to a bouquet is already in the code and predates adoxa's changes.

What I'm saying is that the GREEN button Should only show "Add to favourites" (in non-Favourites bouquets) if pressing GREEN will actually add the service to Favourites, that is, the service isn't already in Favourites.

If that's implemented consistently, that's all that is needed to blank "Add to favourites" from the GREEN button when a service is added to Favourites. That is not implemented in the current code.

Sorry, my bad. When I said "currently", I mean that in context of what adoxa's patch is currently doing (based on his description). It blanks the hint on the GREEN button if the favourite is already added.

He went on to say that he'd remove the code within his patch that *temporarily* changed (until the users navigates off that service) the GREEN button's hint text to "ADDED!" in favour of simply blanking it immediately.

[prl]

Sorry, at this stage I don't have access to a system that I could test with, so I can't be more specific. Now that I think about it a bit more, it may have been caused by exiting the wizard early, which can be done with the exit key (possibly used multiple times to exit the sub-wizards).

From memory, the issue may be reproducible if you stop enigma2 and clear/delete some of the config files in /etc/enigma2, then start it up again.

Sorry, for such vague info... I feel guilty, but I don't have the means to provide anything specific right now.

OK, I'll try mistreating the startup wizard a bit more

[prl]

Yes, the fact that you can't add the same service twice to a bouquet is already in the code and predates adoxa's changes.

What I'm saying is that the GREEN button Should only show "Add to favourites" (in non-Favourites bouquets) if pressing GREEN will actually add the service to Favourites, that is, the service isn't already in Favourites.

If that's implemented consistently, that's all that is needed to blank "Add to favourites" from the GREEN button when a service is added to Favourites. That is not implemented in the current code.

Sorry, my bad. When I said "currently", I mean that in context of what adoxa's patch is currently doing (based on his description). It blanks the hint on the GREEN button if the favourite is already added.

He went on to say that he'd remove the code within his patch that *temporarily* changed (until the users navigates off that service) the GREEN button's hint text to "ADDED!" in favour of simply blanking it immediately.

That might have been what was intended, but this isn't all that explicit:

I would have thought it would be sufficient to just blank the button as soon as the service is added to favourites rather than having this intermediate "ADDED!" state.

It crossed my mind, but ended up as it is; I'll change it.

[peteru]

I think I've got the bug reproduced, but the symptoms are more subtle.

init 4
mv /etc/enigma2 /etc/enigma2.disabled
init 3

Keep using EXIT at every opportunity to dismiss all wizards.
Press FAV or EPG.
Notice the title of the current bouquet. This time around I see Terrestrial LC (TV), but in the past I saw parts of strings that looked like a database query. Probably a case of using un-initialised memory when there are no valid entries found.

[adoxa]

[...] [C]an you please split the work in such a way that there are several smallish pull requests [...]

Sorry, I ended up doing it as one request, after all - four one-line commits, plus a bigger one.

To clarify: GREEN will be "Add to Favourites" if the current service can be added, blank otherwise (it's already present, or is a bouquet). When a service is added the label is immediately blanked.

The construction of the "Choose bouquet" menu in MENU>Add service to bouquet should also be smarter and only show the bouquets that the service can be added to [...]

Would you like me to work on that?

[prl]

Notice the title of the current bouquet. This time around I see Terrestrial LC (TV), but in the past I saw parts of strings that looked like a database query. Probably a case of using un-initialised memory when there are no valid entries found.

I've just been doing the scans from a factory reset with the antenna disconnected. As I mentioned, I've seen the terrestrial TV lcn bouquet being labelled as "Terrestrial LCN".

That's fairly easy to explain.

If there are no TV LCNs in lcndb, userbouquet.terrestrial_lcn.tv is not written. Similarly, if there are no radio LCNs in lcndb, userbouquet.terrestrial_lcn.radio is not written.

If there are no servicerefs at all in lcndb, neither is written, so the terrestrial lcn bouquets used are as in the factory defaults: no radio lcn bouquet, and an empty TV lcn bouquet called "Terrestrial LCN" (and not "Terrestrial TV LCN").

I'm not sure how the service reference query string gets in there, but I could believe it.

[prl]

To clarify: GREEN will be "Add to Favourites" if the current service can be added, blank otherwise (it's already present, or is a bouquet). When a service is added the label is immediately blanked.

Sounds perfect.

The construction of the "Choose bouquet" menu in MENU>Add service to bouquet should also be smarter and only show the bouquets that the service can be added to [...]

Would you like me to work on that?

I'm currently looking at the service scan issues peteru raised. You've probably just implemented most of the code needed to implement the menu changes, anyway. I'd have to wait for the pull request to come through.

There are some slightly tricky bits around the edges, though. At the moment, if there is only one bouquet and the services are being added from somewhere like All services, then the destination bouquet popup doesn't appear. With my suggested changes, the test for this should probably be on whether there is more than one bouquet, rather than whether there is only one bouquet that it can be added to. This probably needs a bit more thought.

I guess that's a long-winded way of saying, yes, please, but perhaps it needs more discussion

[prl]

I think I've got the bug reproduced, but the symptoms are more subtle.

init 4
mv /etc/enigma2 /etc/enigma2.disabled
init 3

Keep using EXIT at every opportunity to dismiss all wizards.
Press FAV or EPG.
Notice the title of the current bouquet. This time around I see Terrestrial LC (TV), but in the past I saw parts of strings that looked like a database query. Probably a case of using un-initialised memory when there are no valid entries found.

I tried this a couple of times. I always got the expected Terrestrial LC (TV).

[peteru]

Did you notice the missing character?

Terrestrial LC (TV) not
Terrestrial LCN (TV)

I think the previous manifestations of this looked very similar to FROM BOUQUET "userbouquet.terrestrial_lcn.tv" ORDER BY bouquet, which would put it in the IniLCNScanner plugin.

[peteru]

which would put it in the IniLCNScanner plugin.

Maybe not. Similar looking strings seem to be all over the place.

[prl]

Did you notice the missing character?

Terrestrial LC (TV) not
Terrestrial LCN (TV)

Yes, that's due to a bit that I missed in my earlier explanation.

/usr/share/enigma2/defaults/userbouquet.terrestrial_lcn.tv not only has a different name for the bouquet than the one generated by IniLCNScanner ("Terrestrial LCN" vs "Terrestrial TV LCN"), the single line with the #NAME in the default is not newline-terminated.

When eDVBDB::loadBouquet(const char *path) loads a bouquet, it "strips the terminating newline" even if there isn't one:
so the unterminated line is read as the C string "#NAME Terrestrial LCN\0" and the code then makes it "#NAME Terrestrial LC\0\0" and it then gets displayed as "Terrestrial TV LC" in the bouquet selection list, and as "Terrestrial TV LC (TV)" in the top left of the screen when the bouquet contents are viewed in ChannelSelection.

Code: Select all

			if ((len = getline(&line, &linesize, fp)) < 2) break;
			/* strip newline */
			line[--len] = 0;

The fix for this issue is to change /usr/share/enigma2/defaults/userbouquet.terrestrial_lcn.tv to contain the correct name in its single "#NAME" line, and to newline-terminate that line.

The code in eDVBDB::loadBouquet(const char *path) should also be fixed, since getline() doesn't necessarily newline-terminate its buffer, even if the line in the file is newline-terminated. That happens when the file line length overflows the buffer capacity argument.

To quote the terminating line of lamedb:

Have a lot of bugs!

[prl]

I think the previous manifestations of this looked very similar to FROM BOUQUET "userbouquet.terrestrial_lcn.tv" ORDER BY bouquet, which would put it in the IniLCNScanner plugin.

I'd very much like to find a way to replicate this so that the cause can be located.

IniLCNScanner seems an unlikely culprit when no services have been scanned, because:

Code: Select all

		lcn.read("TV")
		if len(lcn.lcnlist) > 0:
			lcn.writeTVBouquet()

It's LCN.writeTVBouquet() that writes the userbouquet.terrestrial_lcn.tv file.

[peteru]

I quick-fixed the missing EOL. In the process I also noticed that data/defaults/bouquets.tv had DOS line endings, so that's fixed too.

I'm sure there's more to it than that, but small steps...

[prl]

I quick-fixed the missing EOL. In the process I also noticed that data/defaults/bouquets.tv had DOS line endings, so that's fixed too.

I'm sure there's more to it than that, but small steps...

The DOS-style line endings in written into bouquet files in ../dvb/db.cpp should probably go, too. And there's also the blind removal of the last character when reading lines from bouquet files in the same source file instead of checking whether the line has a newline terminator or not.

[adoxa]

The construction of the "Choose bouquet" menu in MENU>Add service to bouquet should also be smarter and only show the bouquets that the service can be added to [...]

Submitted.

[prl]

I saw. Thanks!

[prl]

I quick-fixed the missing EOL. In the process I also noticed that data/defaults/bouquets.tv had DOS line endings, so that's fixed too.

I'm sure there's more to it than that, but small steps...

The DOS-style line endings in written into bouquet files in ../dvb/db.cpp should probably go, too. And there's also the blind removal of the last character when reading lines from bouquet files in the same source file instead of checking whether the line has a newline terminator or not.

Fixes submitted. Also changed the #NAME entries in the default bouquet files to reflect the #NAME entries written by the code.