Added Planar power controller#153
Conversation
ppodhajski
force-pushed
the
feature/powerController
branch
2 times, most recently
from
d4ab5e5 to
db099ca
Compare
| /** | ||
| * The power state of Planar displays. | ||
| */ | ||
| enum class screenState |
| ++_windowCounter; | ||
| } | ||
|
|
||
| void LoggingUtility::powerStateChanged(screenState state) |
| if (_planarController->getState() == screenState::OFF) | ||
| if (!_planarController->powerOn()) | ||
| put_flog(LOG_INFO, | ||
| "Could not power on the screens" |
There was a problem hiding this comment.
"Could not power on the screens by " << and don't forget to add a space before "
| { | ||
| if (state == screenState::ON) | ||
| return "ON"; | ||
| else if (state == screenState::OFF) |
There was a problem hiding this comment.
else not needed - should be a switch normally
| bool powerOn(); | ||
|
|
||
| signals: | ||
| void powerStateChanged(screenState state); |
| #include <QSerialPort> | ||
| #include <QTimer> | ||
|
|
||
| class PlanarController : public QObject |
| _serial.write("OPA1DISPLAY.POWER=ON\r"); | ||
| return _serial.waitForBytesWritten(serialTimeout); | ||
| } | ||
| else |
There was a problem hiding this comment.
else not needed, there is a return above. You could also format it with an early out:
if(!_serial.isOpen())
{
put_flog(...);
return false;
}
_serial.write("OPA1DISPLAY.POWER=ON\r");
return _serial.waitForBytesWritten(serialTimeout);
But is the isOpen() test still needed now that you added a throw in the constructor?
There was a problem hiding this comment.
Not at all, I wanted to discuss which one we choose.
ppodhajski
force-pushed
the
feature/powerController
branch
from
0772032 to
2cc752c
Compare
ppodhajski
force-pushed
the
feature/powerController
branch
from
2cc752c to
d36d612
Compare
ppodhajski
force-pushed
the
feature/powerController
branch
from
4f12bdd to
e8c2a2b
Compare
| return "UNDEF"; | ||
| case ScreenState::UNDEF: | ||
| return "UNDEF"; | ||
| default: |
There was a problem hiding this comment.
you can combine the default and ScreenState::UNDEF cases
| QString output(_serial.readLine()); | ||
| output = output.trimmed(); | ||
| screenState previousState = _powered; | ||
| ScreenState previousState = _powered; |
| put_flog(LOG_INFO, | ||
| "Could not power on the screens" | ||
| "touching the wall"); | ||
| if (_planarControllerEnabled) |
There was a problem hiding this comment.
no need for an extra _planarControllerEnabled variable, you can just check if(_planarController)
| if (!_planarController->powerOff()) | ||
| if (_planarController->powerOff()) | ||
| { | ||
| DisplayGroupController controller(*_displayGroup.get()); |
There was a problem hiding this comment.
no .get() on smart ptrs!! also can be on a single line: DisplayGroupController(*_displayGroup).hidePanels();
2 participants
No description provided.