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.

Sign up for GitHub

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

Jump to bottom

OP printer settings dialog does not query fresh settings #532

Closed
davidzwa opened this issue May 2, 2021 · 7 comments · Fixed by #545
Closed

OP printer settings dialog does not query fresh settings #532

davidzwa opened this issue May 2, 2021 · 7 comments · Fixed by #545
Assignees
@NotExpectedYet
Labels
bug Something isn't working / good issue report fixed on dev This issue has been fixed and is on its way
Projects
1.2 - Patreon Payback and the Great C...

Comments

@davidzwa
Copy link
Contributor

davidzwa commented May 2, 2021 •
edited

  • Description
    Upon editing settings we gotta have the latest settings set from OP. Fix that with an OP API call. Also add a loader to printer settings dialog.

  • Repro
    Open printer settings dialog.
@davidzwa davidzwa added the bug Something isn't working / good issue report label May 2, 2021
@NotExpectedYet NotExpectedYet self-assigned this May 3, 2021
@NotExpectedYet NotExpectedYet mentioned this issue May 3, 2021
Merged
20 tasks
NotExpectedYet added a commit that referenced this issue May 3, 2021
@NotExpectedYet
Initial Printer Settings class refactor. Fixed #532
a76df7d
@NotExpectedYet NotExpectedYet linked a pull request May 3, 2021 that will close this issue
Feature/printer settings dialog improvements #545
Merged
20 tasks
@davidzwa
Copy link
Contributor Author

davidzwa commented May 9, 2021

It now queries the settings, but its not passed on to the UI.

09/05/2021, 10:09:18 | INFO | OctoFarm-State | Attempting to connect to API: api/system/commands | http://192.168.1.177 | timeout: 10000
09/05/2021, 10:09:18 | INFO | OctoFarm-State | Successfully grabbed System Information for...: http://192.168.1.177
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Attempting to connect to API: api/settings | http://192.168.1.177 | timeout: 10000
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Detected Pi Support!
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Attempting to connect to API: api/plugin/pi_support | http://192.168.1.177 | timeout: 10000
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Got from endpoint: data:{"model":"Raspberry Pi 4 Model B Rev 1.2","model_unrecommended":false,"throttle_state":{"current_issue":false,"current_overheat":false,"current_undervoltage":false,"past_issue":false,"past_overheat":false,"past_undervoltage":false,"raw_value
":-1}} |

@NotExpectedYet
Copy link
Member

It now queries the settings, but its not passed on to the UI.

09/05/2021, 10:09:18 | INFO | OctoFarm-State | Attempting to connect to API: api/system/commands | http://192.168.1.177 | timeout: 10000
09/05/2021, 10:09:18 | INFO | OctoFarm-State | Successfully grabbed System Information for...: http://192.168.1.177
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Attempting to connect to API: api/settings | http://192.168.1.177 | timeout: 10000
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Detected Pi Support!
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Attempting to connect to API: api/plugin/pi_support | http://192.168.1.177 | timeout: 10000
09/05/2021, 10:09:35 | INFO | OctoFarm-State | Got from endpoint: data:{"model":"Raspberry Pi 4 Model B Rev 1.2","model_unrecommended":false,"throttle_state":{"current_issue":false,"current_overheat":false,"current_undervoltage":false,"past_issue":false,"past_overheat":false,"past_undervoltage":false,"raw_value
":-1}} |

Should be able to sort without killing anything major. It just needs to call the cleaner again on the system one, or update the cached. Think I have a graceful work around after your comments anyway.

@NotExpectedYet
Copy link
Member

NotExpectedYet commented May 13, 2021 •
edited

Wasn't a cache update issue, it ran perfectly fine. It was me not pulling the correct information. Port preference comes from my getState command, so it needed to fire: getState, getSettings, getSystem.

Has been fixed in latest PR. Just need to sort the button and write the test now.

I created a new function for this anyway within state, this also calls the printer clean just encase there are any weird timing issues with the default timer. Ran a load of tests and now correctly pulls the new settings.

NotExpectedYet added a commit that referenced this issue May 13, 2021
@NotExpectedYet
Fixed #532 - Now correctly updates octoprint settings
6467c2b
@davidzwa
Copy link
Contributor Author

davidzwa commented May 13, 2021 •
edited

Nice job! What in PrinterClean does it call, like a whole refresh?

Oh btw, we are conflicting on PrinterClean most probably. I will try and minimize my PrinterClean changes on my bugfix PR #568, I propose for you to do the same.
[Edit] did the reduction, wasnt too bad after all.

@NotExpectedYet
Copy link
Member

I shouldn't have changed printerClean here? If I did I'll undo it as I shouldn't need to.

PrinterClean.generate function, it will run the cleaner for a single printer you pass to it so it now reruns that. In my tests the 500ms timer had mostly run already like by the time the updated settings had been called it was just to make sure.

@NotExpectedYet
Copy link
Member

No your right I did aha. I tried to use the cache file, is that complete? It kept returning undefined for me so I updated the printer function to be able to return a single printer as well as the whole bunch.

@davidzwa
Copy link
Contributor Author

Ah my goof there, no that's a work-in-progress :S

@davidzwa davidzwa added the fixed on dev This issue has been fixed and is on its way label May 16, 2021
NotExpectedYet added a commit that referenced this issue May 29, 2021
@NotExpectedYet
Feature/printer settings dialog improvements (#545)
81dcb7b 
* version 1.1.13-hotfix

* remove useless code

* Initial Printer Settings class refactor. Fixed #532

* seperated try/catches

* missing script import

* #Fixed 351

* add power reset listener

* button... doing... nut in

* Squash printer settings dialog

* Fixed #532 - Now correctly updates octoprint settings

* Fixed #576
Fixed #577

* Fix for event listener not getting applied to save button

* added checks for octofarm successful/unsuccessful update

* allow Click Me button to refresh settings page.

* clean up and test init

* Add auth mock

* Small whoopsie

* introduce a prettier file to hopefully stop the random commit changes

* jest test for 400 and 500 errors returned from update settings endpoint

* Prettier

* Changed prettier and add + run command

* move refresh button so can be pressed at anytime
fix issue with port preference been null

* Prettier and gulp source maps

* Prettier on already changed file

* Prettier Ignore more specific client code

* Prettier new file

* Prettier on printer management runner

* Split up getById and list (v2 CRUD style)

* Fix le tests

* Do my job properly this time

* make settings saving refresh dialog
unify the response into a single button
removed 900 check not applicable anymore
neaten up some styling

* remove uneeded refresh calls

Co-authored-by: David Zwart <davidzwa@gmail.com>
@davidzwa davidzwa moved this from In progress to Done in 1.2 - Patreon Payback and the Great Cleanup Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@NotExpectedYet NotExpectedYet

Labels
bug Something isn't working / good issue report fixed on dev This issue has been fixed and is on its way
Projects
No open projects
1.2 - Patreon Payback and the Great C...
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature/printer settings dialog improvements OctoFarm/OctoFarm
2 participants
@davidzwa @NotExpectedYet