Problems with IceTV fetch when IceTV is disabled

[prl]

While I was looking at the crash in IceTV plugin Show log, I found another odd bug.

If MENU>IceTV>Fetch EPG and timer updates now is selected when IceTV is disabled, the IceTV fetch runs. This probably shouldn't happen, because it can overwrite (parts of) the FTA EPG, and because disabling IceTV switches off the IceTV genre lists, you also get a flurry of spurious errors in the IceTV log complaining about non-existent problems with the genre mapping, like this:

Code: Select all

2021-09-24 10:43:08: [EPGFetcher] ERROR: lookup of 0x11 "Crime" returned "Movie"
2021-09-24 10:43:08: [EPGFetcher] ERROR: lookup of 0x11 (remapped to 0x1c) "Murder" returned "Movie"
2021-09-24 10:43:08: [EPGFetcher] ERROR: lookup of 0x11 (remapped to 0x1b) "Mystery" returned "Movie"

A clean solution to this requires updating the list shown in the IceTV menu, and so I've also changed the Enable IceTV/Disable IceTV entries so that only the relevant one shows.

The only downside to the Enable/Disable change is that in the current code, Disable IceTV alone will clear the EPG if IceTV is disabled, and Enable IceTV will clear the EPG if IceTV is enabled. I don't think that many people were aware of that (I wasn't until yesterday), so the usual advice that to clear the FTA EPG you can enable/disable IceTV remains unchanged.

This is what the two versions of the menu look like with the changes. When IceTV is disabled:

Screen Shot 2021-09-24 at 11.13.30.png (58.44 KiB) Viewed 1684 times

And when IceTV is enabled:

Screen Shot 2021-09-24 at 11.09.37.png (58.09 KiB) Viewed 1684 times

Selecting Enable IceTV to enable fetch does nothing.

The problem could also be fixed by having Fetch EPG and update timers now pop up an error box if it's selected when IceTV is disabled, but I think that changing the entry text and function is cleaner.

I'm happy to have any wording suggestions.

[peteru]

The two alternative menu structures could be:

Code: Select all

When IceTV is Disabled:
------------------------------------
1. Show log
2. Login to IceTV server
3. IceTV setup wizard
4. Enable IceTV


When IceTV is Enabled:
------------------------------------
1. Show log
2. Login to IceTV server
3. IceTV setup wizard
4. Disable IceTV
5. Fetch EPG and update timers now

Some of the positive aspects of this arrangement are:

• First option is probably the most commonly used one.
• The next two options are also sorted by their expected likelihood of use.
• The first three options do not change between the alternative menus and retain the same keyboard shortcuts.
• Enabling/Disabling IceTV has the same keyboard shortcut, thus behaving as toggle.
• The Fetch option is only visible when relevant and does not map onto a no-op when disabled.
• The Fetch option can be quickly accessed by a single up arrow keypress.
• Telling the difference between the two menus is easy as the visual clues are formed by the text as well as the number of entries. i.e. the longer menu is shown when IceTV is enabled.

[IanL-S]

The two alternative menu structures could be:

Code: Select all

When IceTV is Disabled:
------------------------------------
1. Show log
2. Login to IceTV server
3. IceTV setup wizard
4. Enable IceTV


When IceTV is Enabled:
------------------------------------
1. Show log
2. Login to IceTV server
3. IceTV setup wizard
4. Disable IceTV
5. Fetch EPG and update timers now

Some of the positive aspects of this arrangement are:

• First option is probably the most commonly used one.
• The next two options are also sorted by their expected likelihood of use.
• The first three options do not change between the alternative menus and retain the same keyboard shortcuts.
• Enabling/Disabling IceTV has the same keyboard shortcut, thus behaving as toggle.
• The Fetch option is only visible when relevant and does not map onto a no-op when disabled.
• The Fetch option can be quickly accessed by a single up arrow keypress.
• Telling the difference between the two menus is easy as the visual clues are formed by the text as well as the number of entries. i.e. the longer menu is shown when IceTV is enabled.

Peter, I like this arrangement - makes things very clear - sometimes I am uncertain if a particular unit is in fact logged in.

Ian

[prl]

I left the order undisturbed as much as possible so people wouldn't have to retrain muscle memory, especially if people use the numbered buttons to select the menu items.

Peteru's suggested reordering would be trivial to do now that the messy bit (changing a ChoiceBox menu on the fly) is done, so there's no technical obstruction to doing it that way..

IanL-S: there are two people called Peter in the discussion

[prl]

Peter[u], I like this arrangement - makes things very clear - sometimes I am uncertain if a particular unit is in fact logged in.

"Logged in" is a different state from "enabled", and my changes only tell you about whether IceTV is enabled or not.

[prl]

This is how peteru's suggested menu layout would look.

IceTV disabled:

Screen Shot 2021-09-25 at 11.46.02.png (42.9 KiB) Viewed 1625 times

IceTV enabled:

Screen Shot 2021-09-25 at 11.46.23.png (46.09 KiB) Viewed 1625 times

[Paul_oz53]

This is how peteru's suggested menu layout would look.

IceTV disabled:


Screen Shot 2021-09-25 at 11.46.02.png


IceTV enabled:
Screen Shot 2021-09-25 at 11.46.23.png

Peteru's suggestion looks good to me.

[prl]

IanSav has made a couple of suggestions off-line: that "Login to IceTV server" should be "Log in to IceTV server", and that since there is a login action, that there might be a case for a logout action (which would be to clear the authentication token in config.plugins.icetv.member.token).

I'm not fussed either way about either of them, though perhaps "Log in" is slightly better.

[peteru]

The existing enigma2 code base does not use the term "Log in" in any user visible text. Please continue using "Login" - it is a commonly used term as both noun and verb.

[peteru]

Although the API for Logout is implemented in the plugin, what is the practical use case for showing it in the user interface?

IMNSHO adding a Logout option just clutters the user interface and presents an opportunity for the user to put the plugin into an undesirable state. If you Logout, you'll pretty soon be presented with the "password required" prompt. You can achieve the same outcome simply by using the existing Login option, which will perform a Logout if there are existing credentials and then attempt a Login to obtain a fresh token.

[prl]

I think that there is some marginal utility in being able to remove the authentication token, at least on the client side. Doing that should also disable IceTV if it is enabled (just as login enables it).

I'm not convinced that there's much call for it, though.

[peteru]

An existing token is disposed of using the Logout API as part of the login process.

Token management should really be performed by the user from the website. The website token management functionality should be protected using stronger authentication than a simple token. The standard industry practice is to present the user with a list of valid tokens together with additional metadata, such as the device model/name, time of token issue, last time the token was used, etc. The user can then make decisions as to which tokens to revoke.

I would strongly advocate against placing any non-essential token management functionality on the client. Enigma2 already suffers from feature creep that makes it hard to use and creates a boatload of corner cases that lead to idiosyncrasies, complications, unexpected side effects and outright bugs. Adding a Logout option does not address any urgent user need. No one has asked for it in the last 7 years and in this instance the suggestion does not appear to arise from an actual user need.

[IanL-S]

Peter[u], I like this arrangement - makes things very clear - sometimes I am uncertain if a particular unit is in fact logged in.

"Logged in" is a different state from "enabled", and my changes only tell you about whether IceTV is enabled or not.

Another seniors moment .... and there was me thinking that 70 was the new 40

Dose not detract from the usefulness of the proposed change

Ian

[prl]

I have now submitted these changes to the source repository.

I needed to wait for other IceTV changes that are now part of the second and third release candidates to be merged before I could submit these changes.