-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Adding Google Service has bug in callback for python3 branch #193
Comments
I'm still not sure how to fix that, but I have been able to work-around. When you get the bad URL you can edit it in the browser url line to remove the :7777 and then go to that link. That will register the google service and there are no issues after that point. |
You are right. It works. |
This code https://github.com/mrworf/photoframe/blob/master/extras/html/index.php needs to be updated to support a non-7777 destination. To support both old an new the whole registration flow would need to change. Essentially the new version should pass the port as well as the IP to the service. The service should assume 7777 if no port is provided, so it can continue to work with the older frames. |
I'll see if I can do that change this weekend since I also would need to restart the service running on my server to enable it. |
Thanks @mrworf ! I had good intentions to jump on the changes you requested, but I also have my Dad visiting for July, so I don't have as much free time as I normally do. Still, I've made some progress. If you're curious, it's the feature/backup branch on my clone at: https://github.com/dadr/photoframe Almost all the work (in progress) is in a new file: load_config.py I have all the basics that I did before, but I've also thought it would be prudent to call the same loading and checking of config values that the normal frame process does. My next step is to figure out what class to instantiate to get that done. I'm also still mulling over what to do if the value testing fails: revert, or warn. It seems that photo frame will fix bad values if it finds them. And then there is whether to test and what to do if the settings being loaded for screen resolution do not show up as a supported value for the current system. <Maybe I'm over-thinking it> |
Sorry for the delay, but backend now supports ports. Still need to edit the google service oauth bit, but should be done very soon. Regarding time, I hear you and no rush, this should be done because we like to, not because it's our job ;-) However, I think it's a bit dangerous to assume that the frame will always be able to auto correct. In some cases maybe that's not even what you want. |
@dadr @JuliAn50M the fix is in on the python3 branch, please verify and let me know if this solved usage of port 80 |
Thank you mrworf! Adding Google service works correctly for me now on port 80. Regarding the the dangerous assumption, do you think it's OK to restore a config and then just try it? If so, I'm really close to having this done. I've been trying to learn how a separate python program could load the right classes so that it could call |
@Snille reported a bug in #189 that adding a google service was still issuing a callback to frameip:7777. I started a branch to fix that, and found a missed item in /extras/html/index.php. But that does not seem to do anything. I'm wondering if I just missed another :7777 somewhere, or whether some code on sensenet has that port hard-coded? If it's the latter, do we need to adjust flask to keep listening to :7777 in addition to port 80 or is it something that would be better to change at sensenet? Also, as I was changing the original :7777 references, I noticed what appeared to be hard-coded messages in the code. Do you think that is "bug worthy?" I don't think you will see the right addresses in on-screen-messages if you use the command line option to listen to a different port.
And @mrworf, while I'm typing, could you comment on whether I'm using GitHub as expected w/r/t pull requests? I find testing easier by doing PRs into my own python3 branch, but that seems to be creating a "moving target" PR into your branch. Is that OK? I fear I'm using the tool incorrectly because it seems to have gotten out of sync. For example, #192 lists a long litany of commits that I made after issuing #187. But in comparing the two current branches visually, I think that the later commits are actually already in your branch, and that #192 only changes Readme and adds the new commit for #189.
The text was updated successfully, but these errors were encountered: