Differential D12456 Diff 45804 src/widgets/atcoreinstancewidget.cpp

Changeset View

Standalone View

View Options

src/widgets/atcoreinstancewidget.cpp

Show All 13 Lines

/* Atelier KDE Printer Host for 3D Printing

GNU General Public License for more details.

You should have received a copy of the GNU General Public License

along with this program. If not, see <http://www.gnu.org/licenses/>.

#include <GCodeCommands>

#include <KLocalizedString>

#include <KNotification>

rizzitelloUnsubmitted

Not Done

Be sure to keep updated includes when rebasing

rizzitello: Be sure to keep updated includes when rebasing

#include <SerialLayer>

#include <QToolBar>

#include "atcoreinstancewidget.h"

AtCoreInstanceWidget::AtCoreInstanceWidget(QWidget *parent):

QWidget(parent)

, m_fileCount(0)

, m_printAction(nullptr)

▲ Show 20 Lines • Show All 344 Lines • ▼ Show 20 Line(s)

374

{

374

m_core.disableMotors(0);

375

m_core.disableMotors(0);

375

}

376

}

376

377

void AtCoreInstanceWidget::handlePrinterStatusChanged(AtCore::STATES newState)

378

void AtCoreInstanceWidget::handlePrinterStatusChanged(AtCore::STATES newState)

378

{

379

{

379

static QString stateString;

380

static QString stateString;

380

switch (newState) {

381

switch (newState) {

381

case AtCore::CONNECTING: {

382

case AtCore::CONNECTING: {

382

m_core.setSerialTimerInterval(0);

383

m_core.setSerialTimerInterval(0);

rizzitelloUnsubmitted

Not Done

I dont like as much when i tried it as the connected notification. Lets try this

add an new private of type AtCore::STATES to atcoreinstancewidget (i called mine m_laststate) then we do this

    case AtCore::IDLE: {
        if(m_laststate == AtCore::CONNECTING) {
            stateString = i18n("Connected to %1", m_core.serial()->portName());
            KNotification::event ...
       }
 .....
    };//end of switch
    m_laststate = newState;
    m_statusWidget->setState(stateString);
}

Then we will only see the dialog if the last state was connecting. This can also help with the error stuff later on too.

rizzitello: I dont like as much when i tried it as the connected notification. Lets try this add an new…

383

m_connectButton->setText(i18n("Disconnect"));

384

m_connectButton->setText(i18n("Disconnect"));

384

m_connectButton->setIcon(QIcon::fromTheme("network-disconnect", QIcon(QString(":/%1/disconnect").arg(m_theme))));

385

m_connectButton->setIcon(QIcon::fromTheme("network-disconnect", QIcon(QString(":/%1/disconnect").arg(m_theme))));

385

m_connectToolBar->setHidden(true);

386

m_connectToolBar->setHidden(true);

386

m_toolBar->setHidden(false);

387

m_toolBar->setHidden(false);

387

stateString = i18n("Connecting...");

388

stateString = i18n("Connecting...");

388

m_logWidget->appendLog(i18n("Attempting to Connect"));

389

m_logWidget->appendLog(i18n("Attempting to Connect"));

389

connect(&m_core, &AtCore::receivedMessage, m_logWidget, &LogWidget::appendRLog);

390

connect(&m_core, &AtCore::receivedMessage, m_logWidget, &LogWidget::appendRLog);

390

connect(&m_core, &AtCore::pushedCommand, m_logWidget, &LogWidget::appendSLog);

391

connect(&m_core, &AtCore::pushedCommand, m_logWidget, &LogWidget::appendSLog);

391

} break;

392

} break;

392

case AtCore::IDLE: {

393

case AtCore::IDLE: {

394

if(m_lastState == AtCore::CONNECTING){

patrickelectricUnsubmitted

Not Done

The indentation level is wrong.

patrickelectric: The indentation level is wrong.

395

KNotification::event("printerIdle"

396

, i18n("Printer is Idle \n Profile %1 on device %2"

rizzitelloUnsubmitted

Not Done

Since this only happens when first connected I think the message should be

"Connected to printer" Instead of "Printer is Idle"

rizzitello: Since this only happens when first connected I think the message should be "Connected to…

397

, m_comboProfile->currentText()

rizzitelloUnsubmitted

Not Done

be sure to indent for the for these continued lines of i18n (here and elsewhere)

, i18n("%1 ...%2"
   , %1 replacement
   , %2 replacement
 )//Closing the i18n call.

rizzitello: be sure to indent for the for these continued lines of i18n (here and elsewhere) ``` , i18n…

398

, m_core.connectedPort())

399

, QString());

400

}

rizzitelloUnsubmitted

Done

case AtCore::IDLE: {
        if(m_laststate == AtCore::CONNECTING) {
            stateString = i18n("Connected to %1", m_core.serial()->portName());
            KNotification::event ...
       }
 .....
    };//end of switch
    m_laststate = newState;
    m_statusWidget->setState(stateString);
}

This way we don't show the notification every time we hit idle state.

rizzitello: ``` case AtCore::IDLE: { if(m_laststate == AtCore::CONNECTING) {…

393

stateString = i18n("Connected to %1", m_core.connectedPort());

401

stateString = i18n("Connected to %1", m_core.connectedPort());

rizzitelloUnsubmitted

Done

This message is poor.

Maybe..

Connected to port %1

rizzitello: This message is poor. Maybe.. > Connected to port %1

394

emit extruderCountChanged(m_core.extruderCount());

402

emit extruderCountChanged(m_core.extruderCount());

rizzitelloUnsubmitted

Not Done

In my tests I have found that if we use

KNotification::event(KNotification::Notification, i18n("Connected"), i18n("Connected to port %1 and using profile %2.",
                                                                                           m_core.serial()->portName(), m_comboProfile->currentText()));

This works with out install, however the configure area is empty.

rizzitello: In my tests I have found that if we use ``` KNotification::event(KNotification::Notification…

rizzitelloUnsubmitted

Done

I think we should do what kde connect does for its notification titles and use the device name something like <Profile Name> (on <device>) for the title. This will let the user quickly scan thru the notification center and see what events happened on what applications (most apps use the program name for the Title)

rizzitello: I think we should do what kde connect does for its notification titles and use the device name…

rizzitelloUnsubmitted

Done

After finishing a print job you get a second connected notification. We need to either move this to Connecting: and make it say connecting or find some way to tell if we are comming from a previous state of Connecting and show the dialog only if that is true.

rizzitello: After finishing a print job you get a second connected notification. We need to either move…

395

m_logWidget->appendLog(stateString);

403

m_logWidget->appendLog(stateString);

396

emit disableDisconnect(false);

404

emit disableDisconnect(false);

397

enableControls(true);

405

enableControls(true);

398

connectExtruderTemperatureData(true);

406

connectExtruderTemperatureData(true);

399

if (m_profileData["heatedBed"].toBool()) {

407

if (m_profileData["heatedBed"].toBool()) {

400

connectBedTemperatureData(true);

408

connectBedTemperatureData(true);

401

}

409

}

402

} break;

410

} break;

403

case AtCore::DISCONNECTED: {

411

case AtCore::DISCONNECTED: {

404

stateString = i18n("Not Connected");

412

stateString = i18n("Not Connected");

405

disconnect(&m_core, &AtCore::receivedMessage, m_logWidget, &LogWidget::appendRLog);

413

disconnect(&m_core, &AtCore::receivedMessage, m_logWidget, &LogWidget::appendRLog);

406

disconnect(&m_core, &AtCore::pushedCommand, m_logWidget, &LogWidget::appendSLog);

414

disconnect(&m_core, &AtCore::pushedCommand, m_logWidget, &LogWidget::appendSLog);

407

m_logWidget->appendLog(i18n("Serial disconnected"));

415

m_logWidget->appendLog(i18n("Serial disconnected"));

416

KNotification::event("printerDisconnected"

rizzitelloUnsubmitted

Done

would you consider this style for the KNotification::event calls ?

KNotification::event("printerDisconnected" // event name
    , i18n("Disconnected")  //Title
    , i18n("Disconnected from %1." //Body 
              , m_comboProfile->currentText() // i18n line substutions
              // if there was a %2 its source would be here                                       
      )// end of the i18n (with vars) 
);// end of notification event.

I find this to be much more readable.

rizzitello: would you consider this style for the KNotification::event calls ? ``` KNotification…

417

, i18n("Disconnected")

rizzitelloUnsubmitted

Not Done

When we connect we only have a title I think disconnect should also have no body since its one line of needed info. If not then we should add a title to the connect one to be consistent.

rizzitello: When we connect we only have a title I think disconnect should also have no body since its one…

418

, i18n("Disconnected from device %1."

419

, m_core.connectedPort()

rizzitelloUnsubmitted

Done

Please use device name not profile

rizzitello: Please use device name not profile

420

)

421

);

408

m_core.setSerialTimerInterval(100);

422

m_core.setSerialTimerInterval(100);

409

m_connectButton->setText(i18n("Connect"));

423

m_connectButton->setText(i18n("Connect"));

410

m_connectButton->setIcon(QIcon::fromTheme("network-connect", QIcon(QString(":/%1/connect").arg(m_theme))));

424

m_connectButton->setIcon(QIcon::fromTheme("network-connect", QIcon(QString(":/%1/connect").arg(m_theme))));

411

m_connectToolBar->setHidden(false);

425

m_connectToolBar->setHidden(false);

412

m_toolBar->setHidden(true);

426

m_toolBar->setHidden(true);

413

enableControls(false);

427

enableControls(false);

rizzitelloUnsubmitted

Done

Disconnected from (PROFILE)

rizzitello: >Disconnected from (PROFILE)

414

connectExtruderTemperatureData(false);

428

connectExtruderTemperatureData(false);

rizzitelloUnsubmitted

Not Done

In my tests I have found that if we use

KNotification::event(KNotification::Notification, i18n("Disconnected"), i18n("Disconnected from %1.", m_comboProfile->currentText()));

This works with out install, however the configure area is empty.

rizzitello: In my tests I have found that if we use ``` KNotification::event(KNotification::Notification…

415

if (m_profileData["heatedBed"].toBool()) {

429

if (m_profileData["heatedBed"].toBool()) {

416

connectBedTemperatureData(false);

430

connectBedTemperatureData(false);

417

}

431

}

418

} break;

432

} break;

419

case AtCore::STARTPRINT: {

433

case AtCore::STARTPRINT: {

420

stateString = i18n("Starting Print");

434

stateString = i18n("Starting Print");

421

m_statusWidget->showPrintArea(true);

435

m_statusWidget->showPrintArea(true);

436

KNotification::event("startPrint"

rizzitelloUnsubmitted

Not Done

This message is awardly worded

Title : "Atelier"
Message: "A print job has started on <device>"

Why device? because i can use the same profile for several printers and as a result the profile name is not a unique identifier.

Ideally This would be be a popup when printing like this and be persistent like a copy job in dolphin

| Print job on <device> X |

Printing: <filename> <Pause button other contols>

estimated time remaining: <estimated time>

<<<<<<<<<<<<<Progressbar>>>>>>>>>>>>>>>>>

and then when finished it would change to be then fade away after a the normal time out

|Print job on <device> X |

Printed: <filename>

Total Job Time: <Print job time>

rizzitello: This message is awardly worded Title : "Atelier" Message: "A print job has started on…

laysrodriguesAuthorUnsubmitted

Not Done

I don't like the idea of a persistent notification if we already have the progress inside the main window. Because a print job can take hours, and the user won't allow a notification on the screen for that long.

laysrodrigues: I don't like the idea of a persistent notification if we already have the progress inside the…

rizzitelloUnsubmitted

Not Done

The persistent notifications like the file transfers for dolphin stay in the tray or notification area and provide visual progress if the application is minimized. the user can toggle if they are shown by clicking the notification icon in the tray area. Im suggesting we do exactly that . If your unsure about how this will function just copy a large file via dolphin and look at how its progress is handled.

If your using several printers from one instance the notifications will be a useful way to monitor the jobs at the same time. As well as allow the user to keep atelier minimized and just get a quick peek at the progress of any job. I really think we should at very least try this and see how it works for us.

rizzitello: The persistent notifications like the file transfers for dolphin stay in the tray or…

437

, i18n("Started!")

rizzitelloUnsubmitted

Not Done

This Event should be removed and a KJob event should instead be used . As stated in previous comments i feel that this notification is redundant since the user must click print to start the job there is no delay to start the job and the UI will reflect that the print job started.

rizzitello: This Event should be removed and a KJob event should instead be used . As stated in previous…

tcanabravaUnsubmitted

Not Done

I don't know the differences from this to a KJob, chris, can you explain a bit?

tcanabrava: I don't know the differences from this to a KJob, chris, can you explain a bit?

rizzitelloUnsubmitted

Not Done

A Kio::job appears to be the type of popup that dolphin uses for the file transfers.
The perfect print pop up (for me) would have the following:

file being printed
the device thats printing
elapsed time / remaining time
A progress bar
An abort and pause / resume button.

rizzitello: A Kio::job appears to be the type of popup that dolphin uses for the file transfers. The…

laysrodriguesAuthorUnsubmitted

Not Done

Idk about that on other platforms than Linux, and that is not related to a notification system, that is about monitoring.

laysrodrigues: Idk about that on other platforms than Linux, and that is not related to a notification system…

rizzitelloUnsubmitted

Not Done

I can not see how this is useful to any users as just a notification that the job started because the user has to click print to start the job and they must have the Application (as well as the specific atcore instance) in Focus and they will see on the GUI the job has started. As I have said before i do not feel this is helpful with out it being a KIO job.

rizzitello: I can not see how this is useful to any users as just a notification that the job started…

rizzitelloUnsubmitted

Not Done

Printing Started.

rizzitello: Printing Started.

438

, i18n("The print job has started on device %1."

439

, m_core.connectedPort()

440

)

441

);

422

connect(&m_core, &AtCore::printProgressChanged, m_statusWidget, &StatusWidget::updatePrintProgress);

442

connect(&m_core, &AtCore::printProgressChanged, m_statusWidget, &StatusWidget::updatePrintProgress);

423

} break;

443

} break;

424

case AtCore::FINISHEDPRINT: {

444

case AtCore::FINISHEDPRINT: {

425

stateString = i18n("Finished Print");

445

stateString = i18n("Finished Print");

426

m_statusWidget->showPrintArea(false);

446

m_statusWidget->showPrintArea(false);

427

disconnect(&m_core, &AtCore::printProgressChanged, m_statusWidget, &StatusWidget::updatePrintProgress);

447

disconnect(&m_core, &AtCore::printProgressChanged, m_statusWidget, &StatusWidget::updatePrintProgress);

428

m_printAction->setText(i18n("Print"));

448

m_printAction->setText(i18n("Print"));

429

m_printAction->setIcon(QIcon::fromTheme("media-playback-start", QIcon(QString(":/%1/start").arg(m_theme))));

449

m_printAction->setIcon(QIcon::fromTheme("media-playback-start", QIcon(QString(":/%1/start").arg(m_theme))));

450

KNotification::event("finishedPrint"

451

, i18n("Finished!")

rizzitelloUnsubmitted

Not Done

Printing Finished.

rizzitello: Printing Finished.

452

, i18n("The print job of device %1 has just finished."

453

, m_core.connectedPort()

454

)

455

);

430

m_logWidget->appendLog(i18n("Finished Print Job"));

456

m_logWidget->appendLog(i18n("Finished Print Job"));

431

} break;

457

} break;

432

case AtCore::BUSY: {

458

case AtCore::BUSY: {

433

stateString = i18n("Printing");

459

stateString = i18n("Printing");

434

emit disableDisconnect(true);

460

emit disableDisconnect(true);

435

m_printAction->setText(i18n("Pause"));

461

m_printAction->setText(i18n("Pause"));

436

m_printAction->setIcon(QIcon::fromTheme("media-playback-pause", QIcon(QString(":/%1/pause").arg(m_theme))));

462

m_printAction->setIcon(QIcon::fromTheme("media-playback-pause", QIcon(QString(":/%1/pause").arg(m_theme))));

437

} break;

463

} break;

438

case AtCore::PAUSE: {

464

case AtCore::PAUSE: {

439

stateString = i18n("Paused");

465

stateString = i18n("Paused");

rizzitelloUnsubmitted

Not Done

We should consider a notification here since using command injection can pause the machine without user interaction.

rizzitello: We should consider a notification here since using command injection can pause the machine…

440

m_printAction->setText(i18n("Resume"));

466

m_printAction->setText(i18n("Resume"));

441

m_printAction->setIcon(QIcon::fromTheme("media-playback-start", QIcon(QString(":/%1/start").arg(m_theme))));

467

m_printAction->setIcon(QIcon::fromTheme("media-playback-start", QIcon(QString(":/%1/start").arg(m_theme))));

442

} break;

468

} break;

443

case AtCore::STOP: {

469

case AtCore::STOP: {

444

stateString = i18n("Stoping Print");

470

stateString = i18n("Stoping Print");

445

m_logWidget->appendLog(stateString);

471

m_logWidget->appendLog(stateString);

446

} break;

472

} break;

447

case AtCore::ERRORSTATE: {

473

case AtCore::ERRORSTATE: {

448

stateString = i18n("Error");

474

stateString = i18n("Error");

449

} break;

475

} break;

rizzitelloUnsubmitted

Not Done

This message Should only show on the first error.. Errors have to be fixed by disconnection currently. This is asking for spam in the notification center due to how errors are sent from the printer.

rizzitello: This message Should only show on the first error.. Errors have to be fixed by disconnection…

laysrodriguesAuthorUnsubmitted

Not Done

Ok, I think that I will remove that notification for now.

laysrodrigues: Ok, I think that I will remove that notification for now.

450

default:

476

default:

451

m_logWidget->appendLog(i18n("Unknown AtCore State, %1", newState));

477

m_logWidget->appendLog(i18n("Unknown AtCore State, %1", newState));

452

qWarning("AtCore State not Recognized.");

478

qWarning("AtCore State not Recognized.");

453

break;

479

break;

454

}

480

}

481

m_lastState = newState;

455

m_statusWidget->setState(stateString);

482

m_statusWidget->setState(stateString);

456

}

483

}

457

484

458

void AtCoreInstanceWidget::checkTemperature(uint sensorType, uint number, float temp)

485

void AtCoreInstanceWidget::checkTemperature(uint sensorType, uint number, float temp)

459

{

486

{

460

static QString msg;

487

static QString msg;

461

switch (sensorType) {

488

switch (sensorType) {

462

case 0x00: // bed

489

case 0x00: // bed

463

msg = QString::fromLatin1("Bed Temperature ");

490

msg = QString::fromLatin1("Bed Temperature ");

464

break;

491

break;

465

492

466

case 0x01: // bed target

493

case 0x01: // bed target

467

msg = QString::fromLatin1("Bed Target Temperature ");

494

msg = QString::fromLatin1("Bed Target Temperature ");

468

break;

495

break;

469

496

470

case 0x02: // extruder

497

case 0x02: // extruder

471

msg = QString::fromLatin1("Extruder Temperature ");

498

msg = QString::fromLatin1("Extruder Temperature ");

472

break;

499

break;

473

500

474

case 0x03: // extruder target

501

case 0x03: // extruder target

475

msg = QString::fromLatin1("Extruder Target Temperature ");

502

msg = QString::fromLatin1("Extruder Target Temperature ");

476

break;

503

break;

477

504

478

case 0x04: // enclosure

505

case 0x04: // enclosure

rizzitelloUnsubmitted

Done

This Line is not in the diff. Spacing got changed.

rizzitello: This Line is not in the diff. Spacing got changed.

479

msg = QString::fromLatin1("Enclosure Temperature ");

506

msg = QString::fromLatin1("Enclosure Temperature ");

480

break;

507

break;

481

508

482

case 0x05: // enclosure target

509

case 0x05: // enclosure target

483

msg = QString::fromLatin1("Enclosure Target Temperature ");

510

msg = QString::fromLatin1("Enclosure Target Temperature ");

484

break;

511

break;

485

}

512

}

486

513

▲ Show 20 Lines • Show All 165 Lines • Show Last 20 Lines