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

EvseV2G: minor fixes, simplify logging #1

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

barsnick
Copy link

@barsnick barsnick commented Mar 19, 2024

DRAFT - DO NOT MERGE

I just want an internal review, before I put up a pull request against upstream.

Describe your changes

The EvseV2G module still uses our old truffle dlog() infrastructure. This has become overkill, as its particular features are unused (file names and line numbers, changing log level at runtime). Furthermore, string concatenations and timestamp calculations are still being done though never used!

This PR strips everything which is not used from the log functions, and converts dlog() from a macro to a function. dlog() is now a simple converter from C-printf-style logging to EVLOG_* streams.

Also, some very minor logic simplifications are made, without impact on the compiled code.

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

Signed-off-by: Moritz Barsnick <[email protected]>
While not converting the module to standard EVerest EVLOG_*, this
significantly simplifies the dlog() cruft to the most necessary bits.

As a side effect, the annoying empty lines on stdout logging are removed.

This also converts the macro to a function.

Also drop obviously obsolete trailing newlines from dlog() calls.

Signed-off-by: Moritz Barsnick <[email protected]>
@barsnick barsnick requested review from mhei, FaHaGit and mooraby March 19, 2024 16:46
@mhei
Copy link
Member

mhei commented Mar 20, 2024

Definitely the right direction. I suggest taking the next step straight away: get rid of dlog completely.

@barsnick
Copy link
Author

get rid of dlog completely

Okay, let's do it.

That will involve using the fmt library, but that's a good thing.

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.

2 participants