search

Rimco / OSPy

A Python port of the Arduino based OpenSprinkler firmware V 1.8.3
10 stars 4 forks source link

Development Chat #24

Open Rimco opened 10 years ago

Rimco commented 10 years ago

This issue should be used as main chat for active contributors. Let me know your ideas and what you want to do. First idea of teodoryantcheff was documented in https://github.com/Rimco/OSPy/wiki/API-idea. Old chat is in https://github.com/Rimco/OSPy/issues/22.

Rimco commented 10 years ago

About the new API. I know that the mobile app wants to support the arduino based version, but I don't want to let that stop us from improving the python version. The current port is largely based on the arduino code. Although this keeps stuff compatible, it is very non-Pythonic. I've already talked to @Dan-in-CA about this. Let me copy some parts of my conversation:

If I look at the program from a high level, I would expect something like the following:

On the short term I think I will improve the output layer to make it more abstract and be able to use a "dummy" output that can be used during development. Currently all try catch constructions are a bit tricky. Next I want to have a look at the plug-in system. I'd like to be able to enable and disable plug-ins from the GUI and get rid of the executable bits. The executable bits now also influence the system update plugin because it has to ignore them to prevent conflicts.

salbahra commented 10 years ago

It is a very fair point and in the end, so long a plugin can expose the data in the method the app understands, things will work fine. So I agree, don't let the mobile app hold you back.

By the way, some of the issues you mentioned are addressed in firmware 2.1 for Arduino.

martinpihrt commented 10 years ago

hello, a) I have a question it would be possible to add something like a watchdog for plugins. If for some reason the plugin crashes and shuts tried by the ospy him again after some time, run, or send an email about a fall or problem b) happens to me quite often that after the update I set the time zone setting OSPy to -12 h and master station is turned off (other settings remain) c) If you are upgrading ospy update using the plugin, I am redirected to the home, but for load changes must reboot the system to load changes thanks

teodoryantcheff commented 10 years ago

Implementing what I've proposed in terms of API only makes sense if there will be a client for this. So if you are not that interested in supporting a new API, despite IMHO way better than the current one, there's not much point in implementing it on the OSP[y|i] side.

Let's first see just how much traction my proposal gets with the community :)

On Sat, Sep 20, 2014 at 8:01 PM, Martin Pihrt notifications@github.com wrote:

hello, a) I have a question it would be possible to add something like a watchdog for plugins. If for some reason the plugin crashes and shuts tried by the ospy him again after some time, run, or send an email about a fall or problem b) happens to me quite often that after the update I set the time zone setting OSPy to -12 h and master station is turned off (other settings remain) c) If you are upgrading ospy update using the plugin, I am redirected to the home, but for load changes must reboot the system to load changes thanks

— Reply to this email directly or view it on GitHub https://github.com/Rimco/OSPy/issues/24#issuecomment-56273568.

Rimco commented 10 years ago

@Martin: a) This might be part of improving the plug-in system. b) If this is a real bug that can be reproduced, you can make an issue for it:) c) The latest System update plugin should do that for you. Although it only works if you use the OSPy service. Do you use that (service ospy start etc.)? Otherwise it might not do that for you. I could make it more robust for these kind of situations.

martinpihrt commented 10 years ago

I use a service ospy ... despite the fact I have to manually apply the settings tab reboot the system to load changes. I have a pure clone of https://github.com/Rimco/OSPy and update doing locally (or via sms and click update)

Rimco commented 10 years ago

That's weird, I also have a plain OSPy clone running and updating (using webpage) works fine. It starts updating, initiates a service ospy restart and lets the client wait for the page to come back. In less than 30 seconds I get a new plug-in page telling me the program is up-to-date. I've created a new branch in which I'll try to improve the overal structure (without impacting the api yet). I'll also include extra logging facilities to be able to debug these kind of problems more easily.

teodoryantcheff commented 10 years ago

@Rimco - re logging, I'd strongly support using this. As good as they get. And another, architectural idea, we can have one logger per plugin and one "system logger".

Rimco commented 10 years ago

That's indeed what I would use and my architectural idea is the same. The system should provide some global logging facility that can also be used by plug-ins. This logging facility should also be able to return the entries per group such that plug-ins can use it as feedback for the user.

teodoryantcheff commented 10 years ago

@Rimco @salbahra : wiki page with API proposal updated. Added programs data format.

salbahra commented 10 years ago

@teodoryantcheff it's almost like you are using my read_program function as the bases :+1: But really thats exactly what my function does and having the firmware do it is obviously a huge help and lowers the barriers to future development. I love what this is shaping up to be. Also, as Dan mentioned, we should get Jonathan in on this conversation since it sounds like he has already started with the development of it.

teodoryantcheff commented 10 years ago

@salbahra - You javascript guys really amaze me! I do understand the language itself, DOM, CSS, HTML5 and all related stuff, but still the way you manage to make it all work together seems like dark-black magic to me. High-five to that ! And haven't looked into your code, actually the whole data structure in programs I "designed" just by looking at the edit program page in the native gui of ospy latest.

salbahra commented 10 years ago

@teodoryantcheff Well I am learning as I go but it's a lot of fun!

I figured, I was just kidding and only meant it abstracts away a lot of the complicated stuff so the UI can focus on it's purpose. The API is essentially only used by the mobile app now and I already have a good idea if you are willing to incorporate it into your proposed API.

Right now, I only support my own plugin in the mobile app. However, if the API allowed, I would be able to manage them in the app and also interact with them (using some sort of agreed format for page formatting). Since I am using jQuery mobile, you could pass a title string and a page content string?

teodoryantcheff commented 10 years ago

@salbahra then you'll like the newly added /plugins/ section :) Very, very raw still, and of course this whole project is a team effort, so whatever is on your mind I want to hear and discuss.

teodoryantcheff commented 10 years ago

@Rimco - regarding the scheduler - this seems interesting.

Rimco commented 10 years ago

Although that might seem a good starting point, all schedules seem to be independent. In our case, we need to be able to combine them intelligently and also predict when they will run. During my conversation with Dan, I also told him I already started implementing such a scheduler before I discovered a Python OSPi existed. Let me copy-paste what I told him:

I've also got some designs lying around and I started coding the basics of the back-end. It contained the scheduler similar to what I described earlier. I designed the program as follows:

First you have "Schedules". Schedules are defined by specifying the active periods. Schedules can optionally repeat after a certain period (1 day, 1 week, 2 weeks, anything). The scheduler can report the time intervals it want to be active given a date/time range. It can also calculate if it would be active at a given moment in time. In a "Program", you can assign a schedule to each output. So multiple outputs can share the same schedule or they can be completely different. The system can have an arbitrary number of programs. (The current OSPy is a specific case of this in which all outputs use the same schedule.) Finally, there is one main "Scheduler". This scheduler maintains a list of programs and it can calculate the combined schedule (taking (weather) adjustments and enabled programs into consideration). In addition, each output should have a "usage" property. This usage property (for sprinklers this could indicate pressure drop or flow) will be used by the scheduler to make sure it will never exceed a set "maximum usage". Thereby the user can enforce sequential programs, but it can also be more intelligent by enabling outputs with low usage at the same time. The scheduler tries to execute each schedule as soon as possible without "breaking the rules". Although finding an optimal schedule is a computational hard problem, it is not necessary to find the "optimal" schedule as long as it provides decent results that the user expects. (E.g. by swapping queued schedules you might be able to combine even more outputs and decrease the total running time.)

I have working implementations of these three parts lying around which I could just copy-paste in the current implementation. My main concern is that this would change the behavior significantly making it incompatible with the arduino implementation (for the mobile app).

Rimco commented 10 years ago

Hi guys,

Please let me know what you think of https://github.com/Rimco/OSPy/blob/refactor/options.py and https://github.com/Rimco/OSPy/blob/refactor/stations.py.

The options instance (from options import options) provides a simple way to get and set options. The predefined options are used for the core functionality. In addition, any object can be saved by giving it to the save function. A similar load function is also available. I think I'm also going to provide some "on change" callback mechanism to notify parts that are interested in a certain property.

You can also use the options instance to save any data persistently. The class makes sure that it will be available whenever the system is restarted. options.test123 = 5 will just work magically;)

Next, the stations implementation makes use of this to magically save station properties. These stations can be used as output instances directly. first_output = stations.get(0) ... first_output.activated = True will do everything for you to enable an output for example.

Let me know what you think, I'm currently trying to figure out which parts that need to be updated to let them work with these new constructions. That seems to be many.

teodoryantcheff commented 10 years ago

Re your first post and the scheduler - The idea that I have -- say we have a program P that states running valve 2 (station 2) for 2 hours starting 15:00 o'clock every day. So the implementation is :

You can see that this architecture is flexible enough to allow all current kinds of scheduling and some additional ones - seasonal schedules, every friday the 13th, "birthday" watering and so on :) . Also the "next" watering cycle is always known. And schedules visualization remains, just as now, an exercise for the UI. Which is where it belongs in my view. In a way that is a view on schedule execution from the perspective of a linked list rather than the perspective of an array. BTW, your view on scheduling as solving for minimal time and not exceeding a certain flow volume is very interesting. Especially the flow limit part. Thanks for sharing !

Finally, as long as the UI can get the same API from the controller, it does make no difference. What I was thinking about this is that the current API (current json API, urls, endpoints and so on) can be implemented as a "legacy" API on top of whatever new we decide to do in ospy.

Will be looking at the new goodies you put up for review shortly, as I have a kid on my hands that needs attention as well :)

teodoryantcheff commented 10 years ago

Ok, comments here since I couldn't find a way to comment code in-line.

options.py
    def __init__(self):
        self._values = {}
        self._write_timer = None

        for info in self.OPTIONS:
            self._values[info["key"]] = info["default"]

        with open('./data/options.json', 'r') as options_file:  # A config file
            self._values.update(json.load(options_file))
def __getattr__(self, item):
    return self._values[item]

since all other case are implicitly taken care of already... i think.

def get_categories(self):
    return sorted(list(set([o.get('category', '') for o in self.OPTIONS])))
stations.py
Rimco commented 10 years ago

Your json loading seems nicer indeed, I've updated it.

The getattr should indeed only be called if it is not found, but the first self._values = {} seems to call it already. I tried to do what you say, but I found out that this way we are certain we can't get in any race condition. Furthermore, we prevent saving any internal variable by accident.

The setattr is always called, so that certainly needs this construction.

I don't know what you mean with "I'd extract the file name to file level or class level "constant"", please elaborate.

Your get_categories throws away the order and also makes a "" category. If an option has no category, it should not be displayed (automatically) on the options page. And the order specified in the options list should be returned as-is. I like one-liners, but this one seems not to achieve the goal I had in mind;)

I've renamed activated to active, but activate and deactivate seem more logical than on and off to me. I "activate" station 1, I don't "on" station 1. The BaseStation state is not a duplication, it's even the only place where the states are saved. The Station instances query the Stations "factory" what their state is. Why? Because a single station might not be responsible for the state handling. Furthermore, by delegating this question to the proper Stations instance, you could also let it query the real output state instead of some buffered state. At the moment I handle this in the BaseStations to keep the subclasses very simple, they only have to make sure that the self._state is used to set the outputs.

teodoryantcheff commented 10 years ago

Thanks :) Re naming -- to put "./data/options.json" as a field or module "constant", but it's a minor thing. setattr -- brainfart on my side.... Didn't know that order is of use, sorry for not asking first. Thanks for the explanation of the design as well and ageed on on/off !

Rimco commented 10 years ago

I've made the filepath a constant:) The order was something I didn't think about either in the first place, no problem;)

@salbahra Do you mean to push OSPy to OSPi or to push the refactor branch to master?

salbahra commented 10 years ago

I actually deleted the comment since its technically not development topic but to OSPi since that's the repo people are using.

Rimco commented 10 years ago

General ideas are also welcome here;) We should check if the OSPi branch has some functionality we need to port. I think Dan kept it fairly in sync. If possible, we might ask Dan to update OSPi with our contents. Currently he is referring developers to this version.

teodoryantcheff commented 10 years ago

Speaking of users -- any idea how big is the userbase of OSPi/y and the arduino variant ? Also what are you guys watering with ospy/i ? I personally have a lawn (3 sets of sprinklers), some vegetables, some thuia trees and some flowers. Overall ~10 "stations" in ospi lingo.

teodoryantcheff commented 10 years ago

Oh, and @Rimco 's changes are quite significant and affecting all of the source. I'd be doing logging otherwise or new API. Now I will have to wait :)

Rimco commented 10 years ago

That's why I'm making my changes in a separate branch. You can use the master branch to make your changes;) Although I may change a lot of things, I'm trying to make it as simple as possible. In the end I could get your parts from master and make them work in the refactor branch.

salbahra commented 10 years ago

@teodoryantcheff Addressing your question of how many user's, I can give you some estimates. Roughly 1k OSPi devices are out in use and I assume a large majority of those users are on Dan's OSPi. I think Github does traffic statistics so maybe Dan can give better insight to the software usage.

The other's use Richard Zimmerman's sprinklers_pi app: http://github.com/rszimm/sprinklers_pi or some self-made solution

Rimco commented 10 years ago

@salbahra you just linked to Rays version with new scheduling options, do you have some concrete information? I couldn't find it. If we know what will be the possibilities, we bight be able to stay a bit in sync.

salbahra commented 10 years ago

Sorry I realized the repo itself isn't very helpful. Try this: https://github.com/OpenSprinkler/OpenSprinklerGen2/blob/master/examples/interval_program/interval_program.ino

Rimco commented 10 years ago

btw @teodoryantcheff, we use it mostly for grass, 4 groups for 300 m^2 in total.

Rimco commented 10 years ago

I've ported my scheduler proof of concept into my branch. It is adjusted to keep it more similar to the current schedule vs program vs outputs design. Have a look at the program and scheduler files:) I think the following example can make things clear:

With one program:

'Program0': { 'enabled': True,
              'manual': False,
              'modulo': 1440,
              'name': 'Program 01',
              'schedule': [[30, 50]],
              'start': datetime.datetime(2014, 9, 21, 0, 0),
              'stations': [0, 1]}

>>> start = datetime.datetime.combine(datetime.date.today(), datetime.time.min)
>>> end = datetime.datetime.combine(datetime.date.today(), datetime.time.max)
>>> scheduler.combined_schedule(start,end)
{0: [[datetime.datetime(2014, 9, 21, 0, 30), datetime.datetime(2014, 9, 21, 0, 50)]], 
 1: [[datetime.datetime(2014, 9, 21, 0, 50), datetime.datetime(2014, 9, 21, 1, 10)]]}

After changing the modulo to 360:

>>> scheduler.combined_schedule(start, end)
{0: [[datetime.datetime(2014, 9, 21, 0, 30), datetime.datetime(2014, 9, 21, 0, 50)], 
     [datetime.datetime(2014, 9, 21, 6, 30), datetime.datetime(2014, 9, 21, 6, 50)], 
     [datetime.datetime(2014, 9, 21, 12, 30), datetime.datetime(2014, 9, 21, 12, 50)], 
     [datetime.datetime(2014, 9, 21, 18, 30), datetime.datetime(2014, 9, 21, 18, 50)]], 
 1: [[datetime.datetime(2014, 9, 21, 0, 50), datetime.datetime(2014, 9, 21, 1, 10)], 
     [datetime.datetime(2014, 9, 21, 6, 50), datetime.datetime(2014, 9, 21, 7, 10)], 
     [datetime.datetime(2014, 9, 21, 12, 50), datetime.datetime(2014, 9, 21, 13, 10)], 
     [datetime.datetime(2014, 9, 21, 18, 50), datetime.datetime(2014, 9, 21, 19, 10)]]
}

Currently, durations can only be given in minutes. Seconds could be added easily, but I didn't get to changing that yet. Also I've simplified the previously mentioned usage to just support sequential mode. (Weather) level adjustments have also been disabled for now in the scheduler.

Furthermore I had to change the options file to a python shelve to support saving the datetime objects without the need to extend the json save/load. If we really need to save it as json we have to enhance it to be able to save these objects.

teodoryantcheff commented 10 years ago

Guys, thanks for the info. @Rimco - what is the name of that design pattern you have applied for station - stations and program - programs (the one you explained when I asked about possible "duplication") ?

[EDIT] comment was entered before the above by Rimco. Also - I've been meaning to ask -- is there any particular reason to have web.py library as part of the source code ?

Rimco commented 10 years ago

@teodoryantcheff It's a sort of factory design pattern:

In object-oriented programming, a factory is an object for creating other objects – formally a factory is simply an object that returns an object from some method call, which is assumed to be "new".[a] More broadly, a subroutine that returns a "new" object may be referred to as a "factory", as in factory method or factory function. This is a basic concept in OOP, and forms the basis for a number of related software design patterns.

I use it to keep track of them more easily and be able to keep them organized. Although I didn't have a particular design pattern up front, I always adjust implementations on-the-fly to keep them as simple as possible. This ended up to be the most elegant/simple solution in this case, but I'm always open to suggestions.

teodoryantcheff commented 10 years ago

It's the part that the created objects reference the factory and vice-verse that evaded me. Other than that I know what a factory pattern is :). Cheers!

Rimco commented 10 years ago

I use that to make the individual objects a bit more intelligent. I'm not sure if it is according to some actual existing pattern;) Certainly making a station "master" should ensure others are not master anymore, that needs help of some more global manager. Same holds for the outputting in shift registers, individual outputs can't do that, the manager can:)

teodoryantcheff commented 10 years ago

It's not a factory for sure :) So regarding web.py being part of the source tree ? And also, regarding the scheduler -- what is the benefit of knowing all the daily on-off times for a program ?

Rimco commented 10 years ago

Ah, now I see your edit;) I don't know why and I don't know for sure how to get rid of it. We might detect that it's not available and install it automatically. Then we could install it globally or create a some copy in a subdirectory. The last option might interfere with the automatic update system.

Or we could require users to install it as a precondition, but I like to keep it simple for users:)

teodoryantcheff commented 10 years ago

OK, will think about it. What about a __getitem__ override in BaseStation ?

teodoryantcheff commented 10 years ago

And also, regarding the scheduler -- what is the benefit of knowing all the daily on-off times for a program ?

Rimco commented 10 years ago

getitem seems a good idea for Stations, I'll have a look at it.

Knowing the on-off time is already needed in many places: It is calculated in the timing loop, it is calculated client-side in JS and it's calculated in the mobile app. By calculating it in a single location, many parts of the implementation can be simplified.

teodoryantcheff commented 10 years ago

Just pushed a raw API, working with dummy data since I think the underlying data structures will change with Rimco's upcoming work. Quick review here and here.

teodoryantcheff commented 10 years ago

BTW, adding a plugin more complex than one single file is ugly right now, taking this into account maybe a good idea whenever the plugins system gets redone.

Dan-in-CA commented 10 years ago

Yes. This has been discussed on the forum. http://rayshobby.net/phpBB3/viewtopic.php?f=28&t=817

You should really take some time to read through the discussions that have taken place over the past year and a half or so.

There is also a documentation wiki that is worth a look: http://rayshobby.net/mediawiki/index.php?title=Python_Interval_Program_for_OSPi

Users have a wide range of skill levels and interests. It is important to keep any changes as smooth as possible for them.

teodoryantcheff commented 10 years ago

@Dan-in-CA - yes, you are absolutely right. I've missed the forum completely. Thanks !

Rimco commented 10 years ago

Hi all,

I think I'm finished with the most crucial parts of the new back-end. Current results:

Stations module that supports RPi, BBB and dummy outputs. Dummy outputs are used as fallback if no other type is available and print info when they are activated. These can be used during debugging to test what would happen.

Station properties:

All station instances are managed by a Stations instance with:

Programs module providing very flexible programs:

Program properties:

All program instances are managed by a Programs instance with "normal" list operations. These ensure programs stay in sync with the persistent storage.

Options class providing options and more generic persistent storage. This class provides convenience functions to load and save data for arbitrary objects. New members can be added by explicitly defining them or by dynamic creation (for plug-ins). All information needed for the options page is also included.

Log module providing a log instance. This module ensures all python logging is captured whenever debug_log is enabled in the options. This should be used by developers or if problems are investigated. This will also ensure all log entries are saved to a human-readable log file. In addition, the log instance provides functions to log events for arbitrary "event_types". For example, a plug-in can log its events using its own event_type. After that, the plug-in can fetch the log entries of that type to present an overview to the user. If debug logging is disabled, information is logged as-is (minimum log level=INFO). With debug logging enabled extra information is added and the minimum log level is lowered to DEBUG automatically.

Events can be generated in two ways:

import logging #standard python module
logging.info('This could be displayed to the user', extra={event_type='Plugin1'})
logging.debug('This will be visible to the user if debug_log is enabled', extra={event_type='Plugin1'}) 
logging.debug('This will only be visible in the log file if debug_log is enabled') 

from log import log #OSPy log instance
log.log_event('Plugin1', 'This could be displayed to the user')
log.log_event('Plugin1', 'This will be visible to the user if debug_log is enabled', logging.DEBUG)

The first option has the advantage that it can add more information (line numbers etc) during debugging. Porting this functionality to the log instance variant is a TODO.

The scheduler module provides all information that is needed to determine which outputs should be activated according to the schedules. The predicted_schedule returns an overview of runs that are predicted to run during the given time range. The time range can also lie in the past, in which case it will return what should have been run according to the current schedule.

The combined_schedule also takes the current time into account. If we are looking at a point in the past, we should use the information from the log. For the future, we look at the prediction. Additionally, we add the currently active runs if applicable.

The check_schedule function performs the actions that should be done whenever automatic mode is enabled. It looks at the scheduled runs and ensures that the corresponding outputs are enabled. It also ensures that the master output is enabled if needed. The master output now also supports a negative master_on_delay. (Master can be activated before actually opening a "normal" output.)

TODO:

This all should be a complete back-end replacement making the overall program better readable and maintainable. I'm not sure up to what extend it will be compatible with the original front-end / interfaces, but I didn't want to let this refactoring be limited by that. The back-end should support at least the original functionality, except that overlapping programs (E.g. 8:00-8:15 and 8:10-:8:30) will now be scheduled properly instead of just skipping the second entry.

Dan-in-CA commented 10 years ago

Sounds really interesting.

Is it supposed to be able to run? I get the following error: NameError: name 'sd' is not defined

Rimco commented 10 years ago

It's not yet supposed to run;) Although the back-end is (somewhat) complete, the front-end needs an update to make use of it. I already started removing some old stuff which has certainly broken the current front-end.

Rimco commented 10 years ago

Btw, I tested the back-end by opening a python shell in the OSPy directory. For example, the following should work:

from options import options
print options
options.debug_log = True

from stations import stations
stations.master = 3
s1 = stations.get(0)
s1.name = 'Output test'
s1.enabled = True
s1.activate_master = True

from programs import programs
programs.add_program()
p1 = programs.get(0)
p1.name = 'New name'
p1.stations = [0]
p1.add(300, 1000)
print p1.schedule

import datetime
current_time = datetime.datetime.now()
check_start = current_time - datetime.timedelta(days=1)
check_end = current_time + datetime.timedelta(days=1)

import scheduler
print scheduler.combined_schedule(check_start, check_end)
scheduler.check_schedule()
Dan-in-CA commented 10 years ago

Cool! I read through your code and it looks very clean and concise. The code spinets you posted above are very helpful to see what it does.