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

support obico #3

Merged
merged 7 commits into from
Feb 17, 2024
Merged

support obico #3

merged 7 commits into from
Feb 17, 2024

Conversation

kennethjiang
Copy link

No description provided.

Copy link
Author

@kennethjiang kennethjiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general the code is quite clean. The biggest problem is the "PhysicalPrinterDialog.cpp". This is a common class shared by all kinds of PrintHost. Ideally we shouldn't add any Obico-specific logic to this file.

//------------------------------------------
namespace Slic3r { namespace GUI {

PrinterCloundAuthDialog::PrinterCloundAuthDialog(wxWindow* parent, PrintHost* host)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the name to CloudPrinterAuthDialog. Please don't misspell "Cloud".


class DynamicPrintConfig;
class Http;
class ObicoPrint : public PrintHost
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the name to be Obico

@@ -415,6 +473,12 @@ void PhysicalPrinterDialog::update(bool printer_change)
if (wxTextCtrl* temp = dynamic_cast<wxTextCtrl*>(printhost_field->getWindow()); temp && temp->GetValue() == L"https://connect.prusa3d.com") {
temp->SetValue(wxString());
}

if (TextInput* temp_input = dynamic_cast<TextInput*>(printhost_field->getWindow()); temp_input) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this part? It looks quite hacky.

@@ -436,6 +500,18 @@ void PhysicalPrinterDialog::update(bool printer_change)
}
}
}

if (opt->value == htObico) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is pretty hacky too.

const PrinterTechnology tech = Preset::printer_technology(*m_config);
if (tech == ptFFF) {
const auto opt = m_config->option<ConfigOptionEnum<PrintHostType>>("host_type");
if (opt->value == htObico) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this part to Obico.cpp or CloudPrinterAuthDialog.cpp? We want to minimize the amount of code specific to Obico in the common classes

@kennethjiang kennethjiang merged commit 5a0f98e into main Feb 17, 2024
12 checks passed
kennethjiang added a commit that referenced this pull request Jul 27, 2024
support obico (#3)

Add printer support for Obico cloud.

---------

Co-authored-by: zzh <[email protected]>
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