Skip to content

Conversation

@marcemmers
Copy link
Contributor

Describe your changes

Issue ticket number and link

Checklist before requesting a review

Copy link
Contributor

@Pietfried Pietfried left a comment

Choose a reason for hiding this comment

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

I was thinking about that one as well, since it only logs on larger messages, but ok for me to reduce log level here as well

@Pietfried Pietfried self-assigned this May 7, 2024
marcemmers added 4 commits May 8, 2024 11:23
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
@marcemmers marcemmers force-pushed the another-libwebsocket-logline branch from 9495fe5 to 2c87ccf Compare May 8, 2024 09:33
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
}

void WebsocketTlsTPM::reconnect(long delay) {
EVLOG_info << "Attempting TLS TPM reconnect with delay:" << delay;
Copy link
Contributor

@AssemblyJohn AssemblyJohn May 8, 2024

Choose a reason for hiding this comment

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

I would like this to be kept, and as info, since there were problems with this in the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We print the exact same thing on line 716. If you think it is better I can move remove the line on 716 and leave this one instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've simply missed that one sorry :). I think this would be better, because if we have a crash or anything in the 'close' this would be the last message printed and would give us a better clue on where the problem might be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will move it to the middle then since it would be a bit strange to print reconnecting and immediately after that print not reconnecting.

EVLOG_info << "LWS connect with info "
<< "port: [" << i.port << "] address: [" << i.address << "] path: [" << i.path << "] protocol: ["
<< i.protocol << "]";
EVLOG_debug << "LWS connect with info "
Copy link
Contributor

Choose a reason for hiding this comment

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

I would keep this as info, it only happens on a connect should not be too spammy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also seems to happen on every reconnect attempt in case the connection does not work. But that is not the reason I removed this one. We also already print the same information in Connecting to uri: etc so I didn't really see a reason to print the same information twice.

Could you elaborate on what you are using from this?

Copy link
Contributor

@AssemblyJohn AssemblyJohn May 8, 2024

Choose a reason for hiding this comment

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

Sometimes there are some info missing from the uri (in certain contexts), maybe extend this one a bit with TPM and remove the second one? I remember missing some port information from the URI that caused me a lot of headaches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do think the second one is more readable to leave in in the future. I will set this back to info for now, there is already a comment above it that it is for debug anyway so will hopefully be removed sometime in the future.

marcemmers added 2 commits May 8, 2024 12:11
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
@marcemmers marcemmers requested a review from AssemblyJohn May 13, 2024 07:04
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
@marcemmers marcemmers merged commit edb8bba into main May 13, 2024
@marcemmers marcemmers deleted the another-libwebsocket-logline branch May 13, 2024 14:46
christopher-davis-afs pushed a commit to US-JOET/libocpp that referenced this pull request May 30, 2024
* Convert another logline to debug in libwebsockets

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Clean up a lot of websocket related logging

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Add log line stating which profile we are connecting to

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Make certificate logline debug as well

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Fix linter

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Move one of the log lines a bit and keep as info

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Small review fixes

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Update lib/ocpp/common/websocket/websocket_libwebsockets.cpp

Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>

---------

Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
drmrd pushed a commit to US-JOET/libocpp that referenced this pull request Jun 4, 2024
* Convert another logline to debug in libwebsockets

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Clean up a lot of websocket related logging

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Add log line stating which profile we are connecting to

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Make certificate logline debug as well

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Fix linter

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Move one of the log lines a bit and keep as info

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Small review fixes

Signed-off-by: Marc Emmers <m.emmers@alfen.com>

* Update lib/ocpp/common/websocket/websocket_libwebsockets.cpp

Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>

---------

Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
Signed-off-by: Daniel Moore <9156191+drmrd@users.noreply.github.com>
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.

5 participants