Improve API of how client should use our library

[PolarGoose]

It is an attempt to elegantly solve #4.

• Have a single callback that returns the entire AxdrCapture object, which includes methods to retrieve a string or float value if needed.
• Add ObisId class

[Copilot]

Pull request overview

This PR updates the library’s public parsing API to use a single per-parser callback that receives the full AxdrCapture, enabling callers to choose how to interpret OBIS/value data (including preserving precision) and avoiding per-parse callback plumbing. It also consolidates logging into utils.h and updates tools/tests to the new API.

Changes:

• Replace the “cooked” callback signature with DlmsDataCallback(std::function<void(const AxdrCapture&)>) and pass it via DlmsParser/AxdrParser constructors; simplify parse() to only take the buffer.
• Introduce AxdrCapture helper methods (obis_as_string, value_as_string, value_as_float_with_scaler_applied, etc.) and remove older utility conversion helpers from utils.
• Refactor logging: move Logger/LogLevel from log.h into utils.h and update call sites; add a shared test log fixture.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tools/decode_readout.cpp	Updated CLI tool to use constructor-injected AxdrCapture callback and parse(data) API.
tests/test_util.h	Adds shared log fixture for tests; used across multiple test files.
tests/test_utils.cpp	Updates to TEST_CASE_FIXTURE and removes tests tied to removed utils helpers.
tests/test_meter_dumps.cpp	Updates integration tests to new parser callback + parse() signature.
tests/test_mbus_decoder.cpp	Switches to fixture-based tests (logging setup).
tests/test_hdlc_decoder.cpp	Switches to fixture-based tests (logging setup).
tests/test_example.cpp	Updates example usage to new callback/constructor and parse() API.
tests/test_axdr_registry.cpp	Updates AxdrParser construction to require callback.
src/dlms_parser/utils.h / utils.cpp	Moves logger into utils.h; removes old float/string/OBIS/hex formatting helpers.
src/dlms_parser/axdr_parser.h / axdr_parser.cpp	Renames capture type to AxdrCapture, adds helper methods, stores callback as member, and emits via member callback.
src/dlms_parser/dlms_parser.h / dlms_parser.cpp	Requires callback in constructor; parse() no longer accepts callback argument.
src/dlms_parser/log.h	Deleted (logging moved into utils.h).
src/dlms_parser/{hdlc_decoder,mbus_decoder,apdu_handler}.cpp	Update includes from log.h to utils.h.
src/dlms_parser/decryption/*	Drops log.h include (now provided via utils.h).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 9 comments.

[PolarGoose]

@Tomer27cz,

This turned into quite a big pull request. You can look at test_example.cpp changes to see how the API changed.
The rest of the changes are basically refactoring to make these API changes work. There are no behavioral changes.

[Tomer27cz]

@PolarGoose

Well now I understand why maintainers want smaller and focued PRs. This is a lot and it would be better to split into multiple PRs.

First of all, what is the reason for the Logger change? This has nothing to do with the API, plus it logs the entire output of the tests to the console. If the tests pass, I dont need to see that. Why was the Logger moved to "utils"? The Logger should stay as it is now.

The adition of ObisId is a good change, but it should be in a different PR. In fact that should be merged first, then API change.

DlmsDataType also needs to be its own PR.

The API change itself is good.

[PolarGoose]

The Logger should stay as it is now.

Ok. Returned logger to the log.h as it was before.

plus it logs the entire output of the tests to the console

True. I didn't know how to ensure that all tests capture logs without boilerplate. It is very useful information for troubleshooting, especially if using AI.
However, I have now found a proper way to do it. Now, logs are printed only if a test fails.

The API change itself is good.

Glad that you like the changes.

Well now I understand why maintainers want smaller and focued PRs

Do you want to pick it up from there? If you want to split it into smaller PRs, I'm fine with that.
What I can offer is that I can try to reference this PR in dlms_meter ESPHome component and see how these API changes fit. Only then can we safely merge this PR.

[Tomer27cz]

Do you want to pick it up from there?

Sure, I can do that. It’s just that a 2,000-line PR is quite overwhelming, and splitting it up into smaller, manageable pieces would be beneficia

[PolarGoose]

It’s just that a 2,000-line PR is quite overwhelming

Ok. The problem is that maybe 10% of this PR can be safely merged into main. 90% of the code is going to break dlms_meter ESPHome component. Then if there is a bug that we want to quickly fix, we will not be able to do that because our main is no longer compatible with ESPHome code.

What I would do:

• Prepare a draft PR of ESPHome dlms_meter component. Check if it works and no other changes are needed.
• Then merge this PR, or if you want, split it into many PR and merge within a couple of days. But we should not have a big gap in merging time. If we break the main branch, we should break it fully, and not stop at 50%.

[Tomer27cz]

Then if there is a bug that we want to quickly fix, we will not be able to do that because our main is no longer compatible with ESPHome code.

Downstream uses stable releases. If we have an urgent bug, we will just create a hotfix branch off the 1.2.0 tag. It's totally fine if main is temporarily broken. (Releases can be made from branches)

I agree that once the PRs are split, we will merge them quickly over a couple of days so main isn't stuck in a half-broken state.

[PolarGoose]

If we have an urgent bug, we will just create a hotfix branch off the 1.2.0 tag.

Yes, we can do that. But the best approach is to keep your main always working and apply all fixes on top of it. But it gets tricky when we have to break backward compatibility. If we prepare properly, we can avoid problems. In our case, we will not need any hotfix branches because we are preparing ESPHome changes simultaneously. Any hot fixes will be applied on top of the new version of the dlms_parser ESPHome component.

We need to prepare ESPHome changes. I will do that.

[Tomer27cz]

Okay we can do it in this PR. I made some changes to clean it up and made the version 2.0.0.

[Tomer27cz]

Will the ObisId::from_string have any use? In ESPHome we convert the obis in __init__.py to an array.

I think we should remove the explicit from ObisId, so std::array<uint8_t, 6> can implicitly convert.

[PolarGoose]

Will the ObisId::from_string have any use? In ESPHome we convert the obis in init.py to an array.

Yes, I might add some weird changes. I'm trying to make the ESPHome component code as simple as possible. But it is an experiment. We can remove them if they don't work out well.

I think we should remove the explicit from ObisId, so std::array<uint8_t, 6> can implicitly convert.

I wouldn't do that. It opens all the cans of worms, because you can accidentally assign something that you didn't intend. Does it help much? Because it increases the chance that we make a mistake. Thus, I don't know.

[PolarGoose]

@Tomer27cz,

I have created a pull request where I integrated these changes:
esphome/esphome#17011

Please take a look when you have time.

[Tomer27cz]

@PolarGoose

I added some comments. Also the ObisId::from_string function is not correct. It doesn't do all of the possible formats of obis code.