i am also noob 
what i always try to do:
- explain everything you do and why you do it that way.
- if code isnt needed for the app then leave it.
- dont use oneliners (they are never self explaining)
- avoid complicated structures if possible
- if you can split things up to smaller blocks they do so
for example this part:
APP_SCHEMA = vol.Schema({
vol.Required(CONF_MODULE): str,
vol.Required(CONF_CLASS): str,
vol.Required(CONF_TIMEFRAME): int,
vol.Required(CONF_ACTION): vol.Schema({
vol.Required(CONF_ENTITY_ID): str,
vol.Required(CONF_SERVICE): str,
vol.Optional(CONF_DELAY): int,
vol.Optional(CONF_PARAMETERS): dict,
}),
vol.Required(CONF_EVENTS): vol.Schema({
vol.Optional(str): vol.Schema({
vol.Required(CONF_ENTITY_ID): str,
vol.Optional(CONF_TARGET_STATE): str,
vol.Optional(CONF_RANK): int,
}, extra=vol.ALLOW_EXTRA),
}),
}, extra=vol.ALLOW_EXTRA)
is really hard to see what it is for, why you use it and what it does exactly.
and also a bit overkill for a simple app like this.
if you use something like voluptuous in such an app, then take the time to give a simple description from what it is, and what it does with a link to the voluptuous docs.
real noobs wont understand why you use:
APP_SCHEMA = APP_SCHEMA
more logical to read is if you put that in the initialise like:
self.APP_SCHEMA = APP_SCHEMA
in general i also try to avoid EVERYTHING that is done outside the class (app code) except imports off course.
why create extra lines?
CONF_ACTION = 'action'
and then later on use the constant. it makes code less readable, because i need to read back every time i see a constant, to know what it stands for. constants like that are nice if you want to use them for translatable output, but not for simple apps like these.
avoid the use from names that are used in a different setting.
a noob knows that attributes can be a part from an entity. now you use it as a part from an event.
better readably is if you call them event_settings.
explain when you add kwars to a listener and when you use them. a noob wont understand how you get to
rank = kwargs[CONF_RANK]
this is probably very correct:
def update_event_trigger_timestamp(self, rank: int) -> None:
but if you look at forums or python scripts you find elsewhere you would probably find:
def update_event_trigger_timestamp(self, rank):
the extra characters are nice if you write for yourself or if you share code, but not for showing options.
in a lot of cases a few extra line will make things more self explaining
def all_events_triggered_in_timeframe(self) -> bool:
"""Returns true if all events triggered within timeframe and in order."""
return (
self.all_events_triggered and
self.in_timeframe and
self.triggered_in_order
)
this works perfectly, but it would be more readable if you use:
def all_events_triggered_in_timeframe(self) -> bool:
"""Returns true if all events triggered within timeframe and in order."""
if self.all_events_triggered and self.in_timeframe and self.triggered_in_order:
return True
else:
return False
i hope this helps you understanding why i said that your code isnt easy to read for noobs.
your code is far away from noob code. you did take a look at advanced coding (and looking at the style, HA code) and use those structures and usage. nothing wrong with that, but its far away from noob code. (nothing wrong with that, unless your goal is to learn others using apps )
