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

feat: Connect to Moonraker via subdirectory/path #1836

Merged
merged 14 commits into from
Apr 27, 2024

Conversation

4cello
Copy link
Contributor

@4cello 4cello commented Mar 30, 2024

Description

This allows to connect to moonraker instances on non-root paths, e.g. all-printers.net/myprinter.

This was already possible by appending the path to the port: 7125/myprinter, which is hacky and non-intuitive.

I added an optional field "path" to the printers. The text fields are hidden in subpanels in SelectPrinterDialog and SettingsPrinterRemoteTab. The only input validation currently is to check that the path starts with a "/", which is in line with validation rules for the hostname.

Things I don't know if they're necessary or how to do (yet)

  • Modify the default webcam path to respect this change: I'm not sure what the most elegant way to do this would be.
  • Moonraker instance database: I don't know if there's any work to do here

Related Tickets & Documents

Addresses #1599 as well as @PSandro's comment on #1163

Mobile & Desktop Screenshots/Recordings

mainsail-printersettings-2024-03-30_11-03
mainsail-editprinter-2024-03-30_11-03

[optional] Are there any post-deployment tasks we need to perform?

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

@4cello 4cello changed the title feat: Ability to connect to Moonraker via subdirectory feat: Connect to Moonraker via subdirectory/path Mar 30, 2024
@meteyou
Copy link
Member

meteyou commented Mar 31, 2024

Thanks for your PR!

Can you only remove the "advanced" accordion in the interface settings dialog and just display path per default with the default value "/"?

And pls use the full width in the small "select printer" dialog for the path input field (when no name field is visible. so when moonraker is used for the instance db)

EDIT: this dialog is not visible when moonraker is the instance DB. so the small dialog should fit.

@4cello
Copy link
Contributor Author

4cello commented Mar 31, 2024

Will do!

What are your thoughts on the accordion in the small dialog? I don't love the look of it in general, but I also think it's good to hide an option which most people don't want to change anyways.

@meteyou
Copy link
Member

meteyou commented Mar 31, 2024

to be honest, i personally don't like this function at all...

maybe you could add a button at the bottom left next to the delete button to show/hide the advanced options?

@4cello
Copy link
Contributor Author

4cello commented Mar 31, 2024

I think it could work, but I'm struggling to find a good icon. The good ol' Expand/Collapse just doesn't feel right when it's below the thing that it controls, and I also think it looks better if there's no text required.

mainsail-collapsed-2024-04-01_01-05

mainsail-expanded-2024-04-01_01-06

mainsail-gear-2024-04-01_01-16

Copy link
Contributor

github-actions bot commented Apr 1, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 5

Copy link
Contributor

github-actions bot commented Apr 1, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

@4cello 4cello marked this pull request as ready for review April 1, 2024 09:37
Copy link
Contributor

github-actions bot commented Apr 1, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

@meteyou
Copy link
Member

meteyou commented Apr 1, 2024

ok. i think i found an issue, when you try to connect to a printer without sub path:
image

…and trailing slashes, then adding back a single slash if resulting path is non-empty
Copy link
Contributor

github-actions bot commented Apr 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

Copy link
Contributor

github-actions bot commented Apr 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

@meteyou
Copy link
Member

meteyou commented Apr 13, 2024

@4cello i added also the feature to use the prefix via config.json (without multi instances) and .env. please doublecheck this commit.

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 4

@meteyou meteyou merged commit 69bed88 into mainsail-crew:develop Apr 27, 2024
10 checks passed
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

Successfully merging this pull request may close these issues.

2 participants