-
Notifications
You must be signed in to change notification settings - Fork 288
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
Remember busspeed #457
base: master
Are you sure you want to change the base?
Remember busspeed #457
Conversation
…ing variables in settings
if (portNames.count() != driverNames.count() || devTypes.count() != driverNames.count()) return; | ||
if (portNames.count() != driverNames.count() || devTypes.count() != driverNames.count() || driverNames.count() != busNums.count()) return; | ||
|
||
// connect tthe bus get/set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: tthe -> the
conn_p->start(); | ||
|
||
connect(this, SIGNAL(updateBusSpeed(int, int)), conn_p, SLOT(updateBusSpeed(int, int))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connect(this, &ConnectionWindow::updateBusSpeed, conn_p, &CANConnection::updateBusSpeed);
Gives errors during compile time instead of runtime.
As discussed in #445 .
It's a bit crude but the bus speed update has to take place after initialization of the connection on most devices hence the different update method. The bus speed is only saved if the change was successful in the configuration screen so for some users it might look like nothing is working due to that failing. My familiarity with the QT Threading system is poor at best, maybe this was not the best way to do it but for me it works and it might be of use to others. I have no way to test it with the current supported devices so YMMV with those implementations.
Not tested on Windows/OSX as I have no working setup for those OS's.