Code review request for new plugin

Having published my first real plugin (I've got drafts of two more of... questionable utility), I'm feeling like it could use another pair of eyes. (It's been a long time since I published code without at least one person doing code review).

If anyone would like to look at https://github.com/nathanjw/ESPHomeClim ... igo-plugin and give me feedback on anything - formatting, Python style, logical errors, holding the Indigo API wrong, etc - I would appreciate it. Replying here would be as fine place to put comments, or DM me.

Thanks!

Are you sure this line is actually doing anything?

Code: Select all

        self.setupFromPrefs(self.pluginPrefs)


Because you have not actually defined or initialized self.pluginPrefs anywhere. There's at least one other usage of this term later on.

Also, when using a .get() call on preferences or such, I prefer to use a second argument for the default value. Often "None", which I then test to make sure I actually got a value.

Also, how is that method of running the asyncio event loop working out? I use a different method to stop the loop. Yours is a little cleaner, but I had never considered using the loop stop() call.

self.pluginPrefs: This does work, though it does seem like it would be better to use the value passed in the function call. The plugin guide section on Device Factories references this member variable, and says "You are not required to have settings here, but a configUI node is required. These will be stored in self.pluginPrefs and will apply to all grouped devices created with the plugin.", which is probably where I got the idea.

Being more defensive with .get() is a fine plan. Done.

The asyncio is working well, including stopping and restarting. I did look at your set of posts on the subject, but I already had this somewhat working and decided to just plow ahead. Getting to where I could catch exceptions and stupid syntax errors in the async methods was tedious, but having both the async exception handler set and adding the Indigo handler to the root logger did the job.

I found that when doing asyncio testing, it's almost required to "restart in interactive shell". That way you see all the asyncio errors as they happen (in the terminal window).