Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
teodoryantcheff opened this issue Sep 18, 2014 · 36 comments
Closed

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

teodoryantcheff opened this issue Sep 18, 2014 · 36 comments

Comments

@teodoryantcheff
Copy link

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 OpenSprinkler/OpenSprinkler-App@7f4101b. Also tried the Chrome 'app'. No go.
With 1.8.3 of OSPy the GUI does work.

@salbahra
Copy link

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
Copy link
Author

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
Copy link

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
Copy link
Author

Ok, will give it a go now.

@salbahra
Copy link

Here is the change log so far for the upcoming 1.2.1: OpenSprinkler/OpenSprinkler-App@1.2.0...master

@teodoryantcheff
Copy link
Author

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

@salbahra
Copy link

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
Copy link
Author

@salbahra
Copy link

Yes.

@teodoryantcheff
Copy link
Author

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 @ 1e471a6

@salbahra
Copy link

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
Copy link
Author

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
#22 (comment).

@teodoryantcheff
Copy link
Author

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
Copy link

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
Copy link
Author

Totally makes sense.!

Cheers,

Teo

@teodoryantcheff
Copy link
Author

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
Copy link
Owner

Rimco commented Sep 19, 2014

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

@teodoryantcheff
Copy link
Author

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
Copy link
Owner

Rimco commented Sep 19, 2014

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

@Rimco Rimco closed this as completed Sep 19, 2014
@salbahra
Copy link

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
Copy link
Author

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

  • What actually got me fiddling with the source code is the fact that I'm working on a custom radio system and wanted to integrate this with ospi. Meaning that the current shift register output "subsystem" is something that I'd like to redesign. Otherwise I will just have to plug my code in a way that does not break anything in the current code base while still allowing me to make use of my radios.
  • The web API - to put it softly currently there is none :) I'm keen on coming up with a proper REST API, but again that will definitely require some quite significant re-work of the current state. On the API in particular I'd like to hear @salbahra 's thoughts as well.
  • The plugins implementation -- while it does work, currently there's no way to enable and disable plugins at runtime and for suckers like me developing on windows disabling plugins is a "pain" since files need to be renamed.

@salbahra
Copy link

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
Copy link

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
Copy link
Owner

Rimco commented Sep 19, 2014

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
Copy link

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

@teodoryantcheff
Copy link
Author

@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
Copy link
Owner

Rimco commented Sep 19, 2014

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
Copy link
Author

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

@teodoryantcheff
Copy link
Author

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
Copy link
Owner

Rimco commented Sep 20, 2014

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
Copy link

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
Copy link
Owner

Rimco commented Sep 20, 2014

The Trello board seems nice, but I'd like to keep it linked to the GitHub project. Let's move all our chatting to #24.

@Dan-in-CA
Copy link

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
Copy link
Owner

Rimco commented Sep 20, 2014

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
Copy link

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

@teodoryantcheff
Copy link
Author

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

  • Native GUIs won't change with this proposed API
  • Whatever API the arduino system has remains as it is
  • @salbahra's OS frontend remains working with the arduino based systems
  • we, the python guys, can make this system better by implementing a better API, which :
    • Will be of help to UI developers, as salbahra has mentioned
    • will ease integration with home automation systems
    • Fix the current spaghetti interface the HTTP interface is (not the GUIs) - salbahra had to add plugins to the python code in order his app to be fully functional
  • Have fun along the way ! :) 👍

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants