-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
feat: Let macros define display options (WIP) #1401
Conversation
i could think of boolean and "slider/range" as possible types for the variable. would you be interested in implementing these two types as well? PS: I love this PR! |
I was already planning on adding a I'd been trying to decide what's best for a Relatedly, I'm thinking I should use I also spent a little bit of time reading up yesterday on vue's reactive model, since that's the only place I'm hitting significant issues. I have zero experience with vue (or really any of these reactive web toolkits) and am still unclear on how to trigger re-rendering of the UI elements when the underlying data changes. I'd appreciate any pointers on that front. |
|
55a8501
to
36ea9a0
Compare
All the core stuff works. Although, I gave up on the slider because it doesn't appear that Vuetify2 can style it correctly (in Vuetify3 it appears to be a simple matter of wrapping it in a The big outstanding thing is that the fields do not dynamically update when the Below is the updated version of the test macro I've been using, which also serves as rough documentation for the format. I'm unclear on where I should put actual documentation for this feature, so a pointer would be appreciated:
|
i just checked the getMacros getter. it only loads the data from the config/settings object of klipper. this is only created once when klipper is started and is not changed afterwards. to be able to dynamically read the current values, you have to read printer['gcode_macro xyz'] and not the config. it may be that i have a short time on monday, then i could extend the getter getMacros to also have access to the current variables. this was simply not used/needed in mainsail until now. |
That doesn't match what I'm seeing, and this line certainly gave me the impression that the macro variables are coming from the printer state: mainsail/src/store/printer/getters.ts Lines 179 to 181 in 8481408
What I'm seeing is that the macro state gets read once at page load. So, if I reload the page any changes I made to the Klipper macro variable get propagated to the UI (without reloading Klipper). However, the UI doesn't update unless I reload the whole page. I did also try pulling |
Ohh! You are right! I overlooked it. I have to check, if mainsail also subscribe it and if the getter works correct. |
Yeah, I was guessing it's something along those lines. I spent quite a few hours digging into the reactive UI stuff, before I figured out that the data updates simply weren't propagating. Then I dug into that for a bit, reading up on vuex and trying to figure out how the state propagates from Moonraker. But that's all quite a bit deeper in the weeds than I was expecting to get, and I don't feel like I have a decent grasp of it. If you have some pointers (ideally an existing pattern in the code that I could follow) then I'd be willing to take another crack at it. |
Ok. I have added a debug output in the incoming Websocket data to doublecheck it. Moonraker doesn't send a update with your macro, but I get updates with this macro: [gcode_macro test_macro2]
variable_test: 0
gcode:
{% set test = params.TEST|default(0)|float %}
SET_GCODE_VARIABLE MACRO=test_macro2 VARIABLE=test VALUE={test} I can remember that Moonraker only updates the printer data 2 levels deep with pushes. The same issue was in the bed_mesh data. |
Gotcha. Guess I'll have poke around at the Moonraker code and get a better sense of how this is handled. Hopefully I can get it to update the state without having to do anything to ugly with macro variables. |
I have tried your code and encoutered two problems: |
@jschuh any update here? |
@meteyou, sorry, I ended up going down the rabbitthole of figuring out how to get Moonraker to propagate the variable updates. But I hadn't quite gotten that sorted out before real life got in the way, and now I just haven't had time to circle back. |
@jschuh thx for your answer! if you need help somewhere, feel free to ping me. maybe we should/could also ask Arksine... |
@meteyou, Sorry, it's really just that I haven't had time to get back to it (and probably won't until at least next month). That stated, it would definitely be helpful to get some feedback from Arksine on propagating the variable updates from Moonraker, because that was the only major sticking point. |
This isn't something that can be resolved in Moonraker. This occurs due to the way Klipper creates "diffs" for status updates. Klipper stores a reference to the dict returned by get_status(), calling The only way to make Klipper detect the change is to call You might be able to get away with creating a variable for each hint. Another alternative is to have one dict for static data, and variables for each value associated with the hints. |
Okay, I have some time to look at this again. Creating a variable for each hint feels hacky, but it is certainly an easy enough solution. That stated, I feel like it would be useful for Klipper to have a more formal way of providing this state to frontends (similar to the EXCLUDE_OBJECTS polygons and the SET_PRINT_STATS_INFO state). I'll take a look at how to best approach this, and post something in the next day or so. |
I'm still thinking through what approach makes sense here and catching up on the related discussion in Klipper PR 6393. |
@jschuh any news here? |
This looks very promissing! |
@jschuh thx for the update! pls let me know, when you have time to continue the work on it. |
I close this PR, because of no response. Pls create a new PR, when you continue to work on this Feature. |
This is a first pass at what I proposed in #1398. It's also my first time doing anything at all with vue.js, so feedback on any of that would be helpful. Here's a picture of what it looks like, and below is the macro I've been using to test:
So, it successfully overrides the parsed parameters with the ones from the macro variable. The big things I know I have left are:
front_end_options
updates inside of Klipper.