9and3r / mopidy-touchscreen

Extension for displaying track info and controlling Mopidy from a touch screen using PyGame/SDL
Apache License 2.0
45 stars 24 forks source link

Added groundwork for supporting a better resolution #7

Closed Syco54645 closed 9 years ago

Syco54645 commented 9 years ago

Great work on this. You compelled me to buy a small touchscreen. Problem was I bought a 7 inch 800x480 touchscreen so the interface was too large. I have laid the groundwork for supporting a better resolution. I intend to finish out this feature as well. If you have any questions feel free to ask!

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.03%) to 18.36% when pulling 3a503ed48a7018a8f76a692ce5627ee84f326c80 on Syco54645:develop into 292379a6387993b55dbf14af8d70b5aa2e59e997 on 9and3r:develop.

ghost commented 9 years ago

This is great! I checked your code on a bigger HDMI screen and now the screen is really adapted to the resolution.

Checking a little bit the code, some changes I would like to make:

Get rid off the hardcoded 8 value I was using. I think it makes more sense to use directly the resolution factor instead (8+self.resolution_factor) and set the default value to 8 or 9. This way is easier to imagine what changes the value. (If you set the value to 10 the screen will be split in 10 zones).

This leads to change (7+self.resolution_factor) and as the bar goes on the down part self.size[1]-self.base_size can be used.

The main screen should be changed. Maybe giving to the main screen the value self.resolution_factor would be helpful.

I really appreciate your work! If you have any question or you need some help don't hesitate to ask.

Syco54645 commented 9 years ago

I agree with that, my goal was to not have the user end up with something unusable by setting the value to 0. Defaults could take care of that but it would be much cleaner to just replace the 8 and have that as a minimum value.

With all of the extra screen real estate that is gained some really cool things could be done. Off the top of my head (all user configurable)

I of course do not want to take the project in a direction that you do not want it to go. I have a pretty busy weekend but will try to squeeze in the 5 minutes of code changes for this pr.

Syco54645 commented 9 years ago

@9and3r I have incorporated your critiques. Check it out and let me know.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.03%) to 18.36% when pulling 9a7c5afa4f77da42b03c7d9a3bc014243ae36974 on Syco54645:develop into 292379a6387993b55dbf14af8d70b5aa2e59e997 on 9and3r:develop.

ghost commented 9 years ago

I have merged your code. I set minimum value to 6 so mopidy will disable the extension and notify the user if they put a value lower than 6. This will be published in v1.0.0.

Regarding the user configurable interface is something that I had in mind. The idea would be to have a text file specifying things like text color, active color, positions of buttons... and set a config value to point the file. I think that changing colors could be easily added but other things should require more work.

Currently I would like to finish v1.0.0 but feel free to tell your ideas to improve mopidy-touchscreen.