kbaskett248 / adafruit_macropad

45 stars 21 forks source link

Make it easier to override default settings #10

Closed kbaskett248 closed 2 years ago

kbaskett248 commented 2 years ago

Overview

This PR makes it easier for others to use this code by providing a way to load a user-specific configuration without changing the versioned code. code.py will first attempt to import a DEFAULT_APP value from a user module. If this import fails, it will import DEFAULT_APP from default_settings.

Users can create a user.py file containing their DEFAULT_APP. Or they can create a user module containing additional apps and add DEFAULT_APP in user.__init__.

avi-perl commented 2 years ago

As a possible alternative approach, I've created https://github.com/kbaskett248/adafruit_macropad/pull/11.

I must say, my idea for a central settings repo did not work out quite as pretty as expected, though it does work.

Perhaps the right approach is what you did, and we can augment it in the future with some way to have the settings overridden by values coming from the host computer (as is done in my original PR). If we moved all the constants into settings as well, those settings could also be overridden in this way, though that does not seem very likely needed.

kbaskett248 commented 2 years ago

Hey @avi-perl, I want to apologize for being silent for a few weeks. Stuff got pretty busy for me.

I took a look at https://github.com/kbaskett248/adafruit_macropad/pull/11. So, first off, I think some sort of settings base class makes sense. Especially since the framework relies on some settings being there; those should probably be attributes instead of keys in a dictionary. With that being said, I think we could keep the logic for how to get those settings internal to the base settings class, so we could still treat it like a dictionary from the outside.

And to your point about moving the constants into the settings, I absolutely think it makes sense to move some of the constants into the settings, like the pixel disable timeout and the colors. Some of the other settings make less sense.

I'll go ahead and merge this PR since we seem to be on the same track here. Then I'll put together what I'm thinking for the settings.