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

[Cleanup/Debug Logs] Use Q_FUNC_INFO in many more places. #337

Merged
merged 36 commits into from
Jan 30, 2024

Conversation

nephros
Copy link

@nephros nephros commented Jan 25, 2024

Make debug info and include guard match source file/function names

@piggz
Copy link
Owner

piggz commented Jan 25, 2024

Thanks! ... could you use Q_FUNC_INFO instead of strings ... i generally do this nowadays :)

@nephros
Copy link
Author

nephros commented Jan 26, 2024

Good point, I'll do some more of those.

@nephros nephros marked this pull request as draft January 26, 2024 08:17
nephros added 6 commits January 26, 2024 09:42
setConnectionState will do it.

Before this, the log would print triple reports like this:

2024-01-25 19:18:14.895 : AbstractDevice::onPropertiesChanged: "org.bluez.Device1" QMap(("Connected", QVariant(bool, false))) ()
2024-01-25 19:18:14.901 : DisConnected!
2024-01-25 19:18:14.901 : void AbstractDevice::setConnectionState(const QString&) "disconnected"
2024-01-25 19:18:14.901 : DeviceInterface::onConnectionStateChanged "disconnected"
@nephros nephros changed the title [Cleanup] BipInfoService->DeviceInfoService [Cleanup/Debug Logs] Use Q_FUNC_INFO in many more places. Jan 26, 2024
@nephros nephros marked this pull request as ready for review January 26, 2024 11:27
@nephros
Copy link
Author

nephros commented Jan 26, 2024

Well, went all over the place with these. Hope this is acceptable.

@jmlich
Copy link
Contributor

jmlich commented Jan 26, 2024

In general, I like that change and I like having function name in the log. However, there is a generic way to do that.
The function myMessageOutput can be adjusted to provide all the details.

void myMessageOutput(QtMsgType type, const QMessageLogContext &context, const QString &msg)
{
QByteArray localMsg = msg.toLocal8Bit();
const char* time = QDateTime::currentDateTime().toString("yyyy-MM-dd hh:mm:ss.zzz").toLocal8Bit();
fprintf(stderr, "%s : %s\n", time, localMsg.constData());
}

I am using that in few projects, for example:
https://github.com/jmlich/geotagging/blob/a332aa865e0e4eae655fa8f998bc63c8ba4a089a/main.cpp#L9-L82

It have some pitfalls, e.g. QT_MESSAGELOGCONTEXT define and some compiler flag must be used, otherwise filename and line is removed.

@nephros
Copy link
Author

nephros commented Jan 29, 2024

The function myMessageOutput [...]

I'm not sure I understand right. Is this a request to switch all qDebug() (and friends) to this custom method?

I could do that of course, but I wouldn't be comfortable doing that without approval from the original code authors/maintainers.

@jmlich
Copy link
Contributor

jmlich commented Jan 29, 2024

Its not request, it maybe suggestion or opening of discussion. As I said previously, I am more less fine with keeping it same as you made it.

@piggz
Copy link
Owner

piggz commented Jan 30, 2024

Ill accept this for now ... ive used the myMessageHandler approach in the past, and maybe something to do as a next step?

@piggz piggz merged commit 9027ec2 into piggz:master Jan 30, 2024
3 checks passed
@nephros nephros deleted the deviceinfo-cleanup branch January 30, 2024 22:17
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.

3 participants