Skip to content

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 <moritz.barsnick@chargebyte.com>
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 <moritz.barsnick@chargebyte.com>
@barsnick barsnick requested review from FaHaGit, mhei 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.

3 participants