Skip to content
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

Create facade before preferences #250

Open
nielsvanvelzen opened this issue Jan 2, 2020 · 3 comments · May be fixed by #260
Open

Create facade before preferences #250

nielsvanvelzen opened this issue Jan 2, 2020 · 3 comments · May be fixed by #260
Labels
Projects

Comments

@nielsvanvelzen
Copy link
Member

@nielsvanvelzen nielsvanvelzen commented Jan 2, 2020

The application talks directly with the preference store and this works fine at the moment. But the keys (and "enum" values) are always strings. This can cause problems when a string is not speller correctly or if a developer assumes a certain value exists but it actually doesn't.

For example: I recently added a "enum" that saves the preferred video player. This could save the string "vlc" or "exoplayer". But what if somehow the value isn't set properly? It could break something!

The idea here is to create a facade that brings all preferences to a single file with typesafety, neatly ordered and documented.

An example implementation (in kotlin) could look like this:

enum class PlayerType {
	AUTO,
	VLC,
	EXOPLAYER,
	EXTERNAL
}

class Preferences(
	private val context: Context
) {
	private val sharedPreferences by lazy {
		PreferenceManager.getDefaultSharedPreferences(context)
	}

	/* Preferences */
	var preferedPlayer by enumPreference("pref_video_player", PlayerType.AUTO)
	var configVersion by intPreference("sys_pref_config_version", 1)

	/** Storage functions */
	private fun intPreference(key: String, default: Int) = object : ReadWriteProperty<Preferences, Int> {
		override fun getValue(thisRef: Preferences, property: KProperty<*>): Int {
			return sharedPreferences.getInt(key, default)
		}

		override fun setValue(thisRef: Preferences, property: KProperty<*>, value: Int) {
			sharedPreferences.edit().putInt(key, value).apply()
		}
	}

	private inline fun <reified T: Enum<T>> enumPreference(key: String, default: T?) = object : ReadWriteProperty<Preferences, T?> {
		override fun getValue(thisRef: Preferences, property: KProperty<*>): T? {
			val stringValue = sharedPreferences.getString(key, null)

			return if (stringValue == null) default
			else T::class.java.enumConstants.find { it.name == stringValue }
		}

		override fun setValue(thisRef: Preferences, property: KProperty<*>, value: T?) {
			if (value == null) sharedPreferences.edit().remove(key).apply()
			else sharedPreferences.edit().putString(key, value.toString()).apply()
		}
	}
}

Working with this can be done in Kotlin:

val preferences = Preferences(context) // Ideally a stored instance in TvApp or something
preferences.preferedPlayer = PlayerType.VLC

Or Java:

Preferences preferences = new Preferences(context); // Ideally a stored instance in TvApp or something
preferences.setPreferedPlayer(PlayerType.VLC);

I think this could improve the way preferences are used and helps new developers to learn what preferences are available and how to use them (if kdoc/javadoc is properly added).

This issue started when @thornbill asked if preference migration was actually useful. The conversation started right here

@nielsvanvelzen

This comment has been minimized.

Copy link
Member Author

@nielsvanvelzen nielsvanvelzen commented Jan 4, 2020

I did an initial investigation into all the preferences currently defined in the code/xml. A lot of them already have a description. I'm waiting for #249 to get merged before actually starting on the code to prevent annoying merge conflicts.

My current plan is to create a new class (like the one in the issue) and adding all preferences to it with good descriptions first. The next step would be to fix all types (for example pref_audio_option should become an enum and pref_max_bitrate and int/long).
The last step would be to revise all preferences and changing the names to be consistent and merge certain preferences (for example #249 for live tv). This last step will probably be in another PR as it might bring up a (long) discussion.

Edit 2020-01-09: Updated document below

System Preferences

A shared preference store with the key "systemprefs" set to private.

Booleans

  • guide_filter_kids
    Preferences.xml:
    Default: false
    Use: Stores the kids filter in the live tv guide
  • guide_filter_movies
    Preferences.xml:
    Default: false
    Use: Stores the movies filter in the live tv guide
  • guide_filter_news
    Preferences.xml:
    Default: false
    Use: Stores the news filter in the live tv guide
  • guide_filter_premiere
    Preferences.xml:
    Default: false
    Use: Stores the premiere filter in the live tv guide
  • guide_filter_series
    Preferences.xml:
    Default: false
    Use: Stores the series filter in the live tv guide
  • guide_filter_sports
    Preferences.xml:
    Default: false
    Use: Stores the sports filter in the live tv guide
  • syspref_audio_warned
    Preferences.xml:
    Default: false
    Use: Will display a message to the user requesting the value for pref_audio_option when set to false.
    Value is changed to true after choosing an option causing this to only display the first time the app is used.

Strings

  • sys_pref_config_version
    Preferences.xml:
    Use: Stores the current revision of the preferences.
    It contains an integer (saved as string) which can be used to check the revision.
    Current revision is 5
  • sys_pref_last_tv_channel
    Preferences.xml:
    Use: Used to store the last chosen live TV channel
  • sys_pref_prev_tv_channel
    Preferences.xml:
    Use: Used to store the previously chosen live TV channel

Shared Preferences

The default shared preference store obtained with getDefaultSharedPreferences().

Booleans

  • pref_alt_pw_entry
    Preferences.xml: ✔
    Use: [TODO]
  • pref_auto_pw_prompt
    Preferences.xml: ✔
    Use: [TODO]
  • pref_bitstream_ac3
    Preferences.xml: ✔
    Use: [TODO]
  • pref_bitstream_dts
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_cinema_mode
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_debug
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_info_panel
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_premieres
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_themes
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_tv_queuing
    Preferences.xml: ✔
    Use: [TODO]
  • pref_enable_vlc_livetv
    Preferences.xml: ✔
    Use: [TODO]
  • pref_live_direct
    Preferences.xml: ✔
    Use: [TODO]
  • pref_live_tv_mode
    Preferences.xml: ✔
    Use: [TODO]
  • pref_live_tv_use_external
    Preferences.xml: ✔
    Use: [TODO]
  • pref_refresh_switching
    Preferences.xml: ✔
    Use: [TODO]
  • pref_send_path_external
    Preferences.xml: ✔
    Use: [TODO]
  • pref_show_backdrop
    Preferences.xml: ✔
    Use: [TODO]

Strings

  • pref_audio_option
    Preferences.xml: ✔
    Value: Enum
    • 0 (Direct)
    • 1 (Downmix to Stereo)
      Default: 0 note: Value is asked on first start
      Use: When set to 1 audio will be downmixed.
      Disables the AC3, EAC3 and AAC_LATM audio codecs
  • pref_auto_logoff_timeout
    Preferences.xml: ✔
    Value: Timeout in milliseconds
    • 1800000 (30 Minutes)
    • 3600000 (1 Hour)
    • 7200000 (2 Hours)
    • 10800000 (3 Hours)
    • 21600000 (6 Hours)
    • 86400000 (24 Hours)
      Use: How long a login is active.
      Only works when pref_login_behavior is set to 0
  • pref_login_behavior
    Preferences.xml: ✔
    Value: Enum
    • 0 (Show login screen)
    • 1 (Automatically login as this user)
      Default: 0
      Use: Behavior for login when starting the app
  • pref_max_bitrate
    Preferences.xml: ✔
    Value: Bitrate in megabit
    • 0 (Auto)
    • 120 (120 Mbits/sec)
    • 110 (110 Mbits/sec)
    • 100 (100 Mbits/sec)
    • 90 (90 Mbits/sec)
    • 80 (80 Mbits/sec)
    • 70 (70 Mbits/sec)
    • 60 (60 Mbits/sec)
    • 50 (50 Mbits/sec)
    • 40 (40 Mbits/sec)
    • 30 (30 Mbits/sec)
    • 21 (21 Mbits/sec)
    • 15 (15 Mbits/sec)
    • 10 (10 Mbits/sec)
    • 5 (5 Mbits/sec)
    • 3 (3 Mbits/sec)
    • 2 (2 Mbits/sec)
    • 1.5 (1.5 Mbits/sec)
    • 1 (1 Mbit/sec)
    • 0.72 (720 Kbits/sec)
    • 0.42 (420 Kbits/sec)
      Default: 0
      Use: Preferred maximum bitrate when watching series/movies.
  • pref_resume_preroll
    Preferences.xml: ✔
    Value: Duration in seconds
    • 0 (None)
    • 5 (5 seconds)
    • 10 (10 seconds)
    • 20 (20 seconds)
    • 30 (30 seconds)
    • 60 (1 minute)
    • 120 (2 minutes)
    • 300 (5 minutes)
      Default: 0
      Use: [TODO]
  • pref_video_player
    Preferences.xml: ✔
    Value: Preferred video player
    • auto
    • exoplayer
    • libvlc
    • external
      Default: auto
      Use: [TODO]

Preferences.xml

All preferences defined in the preferences.xml file.

Acra

Not mentioned in code because the library searches the values by itself. See Letting your users control ACRA for more information about these preferences.

  • acra.alwaysaccept
  • acra.enable
  • acra.syslog.enable

Other

These are all documented above

  • pref_alt_pw_entry
  • pref_audio_option
  • pref_auto_logoff_timeout
  • pref_auto_pw_prompt
  • pref_bitstream_ac3
  • pref_bitstream_dts
  • pref_enable_cinema_mode
  • pref_enable_debug
  • pref_enable_info_panel
  • pref_enable_premieres
  • pref_enable_themes
  • pref_enable_tv_queuing
  • pref_enable_vlc_livetv
  • pref_live_direct
  • pref_live_tv_category
  • pref_live_tv_mode
  • pref_live_tv_use_external
  • pref_login_behavior
  • pref_max_bitrate
  • pref_playback_category
  • pref_refresh_switching
  • pref_reporting_category
  • pref_resume_preroll
  • pref_send_path_external
  • pref_show_backdrop
  • pref_video_player
@nielsvanvelzen nielsvanvelzen linked a pull request that will close this issue Jan 9, 2020
@nielsvanvelzen

This comment has been minimized.

Copy link
Member Author

@nielsvanvelzen nielsvanvelzen commented Jan 9, 2020

PR with initial progress opened. It only covers the very basics now.

@nielsvanvelzen

This comment has been minimized.

Copy link
Member Author

@nielsvanvelzen nielsvanvelzen commented Jan 9, 2020

Current progress (tasks may be added):

  • Create an abstract storage class
  • Implement storage class for system preferences
  • Implement storage class for user preferences
  • Use newly created storage classes in code
  • Add migration
  • (maybe) use own store implementation in settings activity/fragment
  • Use integers/long instead of strings for types where it is expected
  • Use enums where possible
  • Fix naming (some keys have strange/wrong names, like "pref_enable_tv_queuing" which is used for normal video playback too)
  • Add documentation (kdoc) to all newly added classes/functions
  • Add documentation for all preferences
@thornbill thornbill added this to In progress in v0.12.0 Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v0.12.0
  
In progress
2 participants
You can’t perform that action at this time.