search

Rimco / OSPy

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

Salbahra's Sprinklers UI cannot login to OSPy latest. #22

Closed teodoryantcheff closed 10 years ago

teodoryantcheff commented 10 years ago

Well the title says it all.

This is what I get in the logs of OSPy upon clicking "Connect Now" in Sprinklers

192.168.0.10:57757 - - [19/Sep/2014 01:11:17] "HTTP/1.1 GET /jp" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:17] "HTTP/1.1 GET /jp" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:17] "HTTP/1.1 GET /jp" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:17] "HTTP/1.1 GET /jn" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /jn" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /jn" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /jo" - 200 OK
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /js" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /js" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /js" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /jc" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /jc" - 303 See Other
192.168.0.10:57757 - - [19/Sep/2014 01:11:18] "HTTP/1.1 GET /jc" - 303 See Other

The 303s redirect to /login

Tried with both latest from repo and this https://github.com/salbahra/Sprinklers/commit/7f4101b597c490cf7315acccdab88cbcad7b10ba. Also tried the Chrome 'app'. No go. With 1.8.3 of OSPy the GUI does work.

salbahra commented 10 years ago

I have already fixed this and will update the app soon.

I don't consider this a bug yet since this version of OSPi is unreleased at this point.

If you want, your welcome to use the PhoneGap Build beta available here: https://build.phonegap.com/apps/793070/install

teodoryantcheff commented 10 years ago

Thanks ! Aren't those built from the exact commit I've referenced -- the 1.2.0 release (7f4101b597c490cf7315acccdab88cbcad7b10ba) your repo ?

And yes - I do agree with it's no bug, simply did not know where to bring this up.

salbahra commented 10 years ago

No, unfortunately, I don't use unique build numbers for each beta/test. I also don't update until before release (instead of running with 1.2.1 until release). It was just the scheme I started with and now too lazy to fix/change.

Thanks for the report though and of course if it still has issues let me know.

teodoryantcheff commented 10 years ago

Ok, will give it a go now.

salbahra commented 10 years ago

Here is the change log so far for the upcoming 1.2.1: https://github.com/salbahra/Sprinklers/compare/1.2.0...master

teodoryantcheff commented 10 years ago

"An IP address is required to continue" no matter what I put in the box. Address, address and port ....

salbahra commented 10 years ago

Okay I added a fix for Arduino and I overlooked the OSPi/OSPy. Can you see if the updated build works now?

Thanks for testing!

teodoryantcheff commented 10 years ago

Same url (https://build.phonegap.com/apps/793070/install) ?

salbahra commented 10 years ago

Yes.

teodoryantcheff commented 10 years ago

Nope. I can see it making the calls :

192.168.0.68:54410 - - [19/Sep/2014 02:45:08] "HTTP/1.1 GET /jo" - 200 OK
192.168.0.68:54410 - - [19/Sep/2014 02:45:08] "HTTP/1.1 GET /jp" - 200 OK
192.168.0.68:54410 - - [19/Sep/2014 02:45:08] "HTTP/1.1 GET /jn" - 200 OK
192.168.0.68:54410 - - [19/Sep/2014 02:45:08] "HTTP/1.1 GET /jo" - 200 OK
192.168.0.68:54410 - - [19/Sep/2014 02:45:08] "HTTP/1.1 GET /js" - 200 OK
192.168.0.68:54410 - - [19/Sep/2014 02:45:08] "HTTP/1.1 GET /jc" - 200 OK

But on the phone I just get returned to the add device page and the input fields are empty.

FYI: running OSPy @ 1e471a655b669f4c9ab9bece2f8ee54c2e2d1df2

salbahra commented 10 years ago

It is working on my end so the best thing I can suggest is trying to use the www files inside my repo in your browser on the desktop.

This will allow you to use the developer console to figure out what's going on. Just a heads up though, you will need to disable web security (otherwise you will get CORS errors).

If we add JSONP support to the mobile_app plugin we avoid the CORS issue. I might consider it at some time.

teodoryantcheff commented 10 years ago

Will do. Tomorrow though - it's getting .. early here :)

On Fri, Sep 19, 2014 at 3:04 AM, Samer Albahra notifications@github.com wrote:

It is working on my end so the best thing I can suggest is trying to use the www files inside my repo in your browser on the desktop.

This will allow you to use the developer console to figure out what's going on. Just a heads up though, you will need to disable web security (otherwise you will get CORS errors).

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

teodoryantcheff commented 10 years ago

in the console :

Uncaught TypeError: Cannot read property '0' of undefined @ main.js:3071

image

web sec disabled with a command line param. Chrome Version 37.0.2062.120 m.

Trying to login to OSPy latest fails returning to :

image

after clicking submit on a filled in form.

Logs in properly on ospi 1.8.3.

salbahra commented 10 years ago

I recommend you start with a clean OSPy. I have wiped my copy and cloned the current copy of OSPy and have no issues. Once you login try to slowly add your settings back and figure out what's breaking it.

Sorry I don't have the information to figure out what specifically is breaking here and too many variables exist to spend time playing myself.

teodoryantcheff commented 10 years ago

Totally makes sense.!

Cheers,

Teo

teodoryantcheff commented 10 years ago

Ok, I got to the bottom of this -- if OSPy sends a longer station names list in /jn than the numbers in ps for /jc then GUI gets confused like I have described previously.

Apparently this condition is relatively easy to get into since the station names list is sent straight from the json "storage" file on the OSPy w/o any validation or sanity checking. It should be sanitized in OSPy for sure, but I think that Sprinklers will also be a better piece of code as well if it manages with such data discrepancies.

Along these lines and very unrelated -- I'd like to see ospy having a real, REST API some day, not the current list of GETs with some params :)

Rimco commented 10 years ago

So the issue for OSPy is that we should do more sanity checking?

teodoryantcheff commented 10 years ago

Not really, no. Though I'm not really sure what is proper course of action here -- on the one hand it is a problem per se, but on the other I can't really see how normal users would get into such a situation. I opened an issue here, since I didn't really know where to discuss.

Rimco commented 10 years ago

Let me at least close it than, we can still talk;)

salbahra commented 10 years ago

Okay I've actually seen this issue before but never investigated it but instead restored OSPi. I think the best thing I can think of is to limit the number of stations to the lowest total stations reported by either "ps" or "snames".

teodoryantcheff commented 10 years ago

And since we've already started talking here's a list of topics I'd like to discuss :

salbahra commented 10 years ago

For the custom radio system, I don't think the core scheduling methods should be changed to accommodate that. It seems completely outside the scope of OSPi and it's not ideal to bend out of the designed use of the software for an edge case like this. I think for this one, the complicated path has to be take unless it's something that also benefits the people using it for sprinklers. Please note, this portion of the code closely mimics how the Arduino OpenSprinkler works. As a result, the mobile app is compatible between the two. If these begin to diverge too much the mobile app might not exist for OSPi. With that said, we are adding some changes to firmware 2.1 (on the Arduino) that I have already started talking with Dan about adding into OSPi. One of the major features is per-station watering times. Rimco, let me know if you want to be involved in adding this and I can share our implementation on Arduino.

The web API exists but isn't documented very well or very consistently. I have setup JSON output's for all of the controller's variables to be used by my app. The second component is the input/change methods which all exist as HTTP requests that don't have to be used via the OSPi web interface (as shown by my app). I think the documentation could help address this issue and is slated for the Arduino and most will be the same on the OSPi, so I will share this when completed. For my purposes though, the API offers complete access to view and change all features in the software.

For the plugins, I think a lot of work went into these and honestly just a few months ago they didn't exist. By the shear number of plugins in existence already, I would say this is a huge success. I think simply adding a plugins collapsible in the options page with an enable/disable flag will solve this issue. All the script has to do is change the permission which disables a plugin from auto loading on next start. Of course, a reboot of the OSPi software is needed.

salbahra commented 10 years ago

The particular issue is in webpages.py inside update_scount. It needs to update gv.snames to the new count. I am updating this now.

Furthermore, after some thinking, I don't think I need a check for this in my mobile app. I really should expect the the OSPy to return the correct/consistent info.

Update: I believe the shortening portion of update_scounts needs this line:

jsave(nlst, 'snames')

Rimco commented 10 years ago

I agree that the radio part should not be part of the core OSPi system. Might some minor part have to change to be able to support a "radio" plug-in, that could still be an option. But I need some more information about this radio part to be able to give a proper advice.

The web API is not very structured or clean (mostly to be backwards compatible), but it currently gets the job done. If we want to provide a proper API, my advice is to create it while also retaining the current interfaces that are used by the app. Eventually, the arduino version might also implement this new API and then the app can switch if wanted.

For the plugins, I've already told @Dan-in-CA that we should get rid of the executable bit. I will personally take a look at this and I'll also try to provide some way to enable/disable them from the GUI.

salbahra commented 10 years ago

Sent a fix under pull request #23. In my quick testing, this seemed to fix the issue.

teodoryantcheff commented 10 years ago

@salbahra - sounds about right, though what you previously said seems like a good idea as well -- base your logic on whatever ospi reports as stations number, not on the number of names in the returned list.

@Rimco - I will explain the radio part in detail if you are interested just after I can back my words with something to show :) Regarding retaining current stuff for compatibility -- absolutely. I will post a proposal here soon for you guys to see and comment. None of this affects the current "native" web part, it's only regarding the API side of things. Finally regarding plugins -- the lazy fix that I apply is just to rename to _[pluginname].py. But that will not work generally since we also have a init file there, which makes this approach ugly.

Rimco commented 10 years ago

I won't change anything on the file system to enable/disable plug-ins. It will be some new option specifying which plug-ins to load. Only thing I need to do is make sure all plug-ins have some way to stop and restart them.

teodoryantcheff commented 10 years ago

I know, it'd be stupid to do so. I was just saying how I cope now :)

teodoryantcheff commented 10 years ago

Here are my thoughts about the API. Still a WIP, but the general idea should be clear: https://github.com/Rimco/OSPy/wiki/API-idea

Rimco commented 10 years ago

Idea seems good, although we might first want to check if the current "collections" have the functionality we want/need. Let's keep this issue a bit clean, I've moved your proposal to https://github.com/Rimco/OSPy/wiki/API-idea . I'm still thinking how we can set up a proper communication channel for development.

salbahra commented 10 years ago

So I agree this is very clear and the data returned is a lot nicer than a ton of bits. Here are my thoughts:

For the Arduino this isn't possible (due to resource limitations) which is why my mobile_app created the current JSON format (you should have seen how I got data before). My goal has of course been centered around supporting both devices. Therefore, as amazing as this (and I agree I think it's the best long term solution) I won't end up using it much personally (so would still need my methods too).

Rimco, would a Trello board help?

Rimco commented 10 years ago

The Trello board seems nice, but I'd like to keep it linked to the GitHub project. Let's move all our chatting to https://github.com/Rimco/OSPy/issues/24.

Dan-in-CA commented 10 years ago

Regarding the API, Don't forget that Jonathan Marsh has been developing the web UI for ospi. He has started to implement an API which can be accesses at [osp url]/api/status.

I think Jonathan should be a part of this discussion.

Dan

Rimco commented 10 years ago

How can we reach him? Please refer him to #24 and continue the chat there:) Will that api also be implemented in the arduino code? Is there any documentation available?

Dan-in-CA commented 10 years ago

I don't think Jonathan is planning to do any work on the Arduino code. No documentation yet.

teodoryantcheff commented 10 years ago

Well, that's cool. Since we already have an Aruino based system and a [put arm board here] based system :

Oh, and if the arduino system has enough horsepower to run a web-site it certainly can do a simple json api :)