can i move log_data() and push_update() into the base classes of the api

[prash3r]

Edit: title: to be clear i dont want to move the functions log_data() and push_update() I just want to move the function calls.

Hi,

is there a reason for the CBPiSensor base class not to take over the logging of the values and the push update functionality? It would take away a lot of complexity and potential problems from the plugin writers. This way we could set a default run() intervall as well as a global parameter for log_data() and it would be harder for the user to spam the data logs because they forgot sleep(1). And the main advantage would be that a failing plugin doesnt fail once and then is just silent but its run() would still be called on the interval and the user would know that there is a problem with this plugin because he gets spammed with the error - for me its more important to know that a plugin fails to run every second then the downside of spammed logs/notifications.

https://github.com/avollkopf/craftbeerpi4/blob/3d72071303377b1855fa05d4eff9f17b6ed32949/cbpi/api/sensor.py#L9

I would like to move push_update() and log_data() into the base class functionality as well as the loop execution and the sleep. So basically moving it all into _run() and calling run() from _run(). So that the run() will be called periodically depending on the running state while the sleep value has a default 1s that can be changed if needed. Problems could be avoided when a plugin programmer forgets to call or spam calls log_data() or push_update(). Also i would like run() errors to be caught so if there is an error it will run the next loop anyway (and spams the log on problems instead of failing silently once - i would want to know this). This approach could be applied to other plugin classes as well but it would basically break all the existing plugins. But i guess its just too much hassle and way too late to change the api now. But if we talk about this: the sooner the better. I mean it would reduce the example DummySensor Plugins run() function from:

async def run(self):

    while self.running:

        self.value = random.randint(10, 100)

        self.log_data(self.value)

        self.push_update(self.value)

        await asyncio.sleep(1)

to just

async def run(self):

    self.value = random.randint(10, 100)

because run() would only be called when while.running is True.

Maybe we could even introduce some kind of api version string into the plugin templates which is checked and depending on that we take over these tasks or we dont so we keep compatibility for the existing plugins but this we need to discuss further, it may creates too much complexity. Alternatively the same thing could be accomplished by just implementing the proposed logic in new classes named differently and change the docs so people would rather derive the new class CBPi4Sensor instead of CBPiSensor. This way we would also keep compatibility to legacy plugins. We dont know who already wrote plugins for their own special case ..

I am happy with a "no its too late, we dont do it" but i would also be happy to be assigned and implement it myself but first we need to talk about how it should be done. I would of course also update the create command and docs including the programmers guide accordingly.

I am hoping for your feedback

[avollkopf]

That might be comlicated for other type of sensors like the scd30 or the ispindle,....

Also here it would be great to chat a bit

[prash3r]

i was thinking about this and if we decide to break (plugin)userspace here: a very simple and lazy way of restoring functionality of existing plugins is to just overwrite the derived _run() with the legacy _run() function from the api.

because all my proposed changes would just happen inside the _run() of the base class. when overwriting this with the current _run() we are back to legacy functionality and all the code is inside the plugin itself.

It would still require changes to every single plugin, but we had a lazy way to migrate existing plugins that we could also publish in the docs and or write pull requests to every existing plugin we can find while just literally copy pasting the original _run() function into the derived class.

Anyway, i just joined the fb group, hit me up anytime!