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

Fix m_bus length calculation #2749

Merged
merged 2 commits into from
Aug 4, 2024
Merged

Fix m_bus length calculation #2749

merged 2 commits into from
Aug 4, 2024

Conversation

rstephan
Copy link
Contributor

@rstephan rstephan commented Dec 7, 2023

Removes "strange" +2/-2 offset from length
Discussion on #2711

Removes "strange" +2/-2 offset from length
Discussion on merbanan#2711
@zuckschwerdt
Copy link
Collaborator

@merbanan can you check what's going on?
The code comes from #768 and was changed in 17738a5

@merbanan
Copy link
Owner

So there is a defacto standard of adjusting the output of the telegram in hardware decoders. This code is supposed to output the same data as hardware decoders. There might be bugs but AFAIK this is correct for wmbusmeters use.

Can you elaborate why you want to change this ? Ie give some telegram output example with before and after.

@rstephan
Copy link
Contributor Author

According to my testing and experiments:

Our water meter (Qalcosonic) uses encryption to get the data.
Therefore a "error free" reception is necessary.
From my understanding wmbusmeter didn't care about the first byte and just count the data afterwards. This is to compensate for "bad" decoders.
Therefor the number of data-bytes must be right.
Due to the encryption, the data must be exact. With extra byes at the end the decryption will always fail.
During initial testing, decoding with rtl_433 and wmbusmeter was not possible, regardless of setting.
After playing a bit with the online-decoder https://wmbusmeters.org/ I have success after adjusting the amount of data.
After the findings, and removing the two "2"s decoding was possible if I could get a packet received.

Right now, due to bad reception in the basement and the holiday season, I am not able to provide example data in a short term.

I can understand if unencrypted data is decoded, some extra byes at the end is not a big deal, but AES does care.

And if the "last" commit 17738a5 to m_bus.c was to make it work with wmbusmeters, this one to make is work again. :-)

I will provide example data next year, but don't expect our AES key.

@rstephan
Copy link
Contributor Author

Here is example data. Both frames are captured at the same time.
wmbusmeters was able to decode the payload, everything after the underscore is the decrypted data.

Data from rtl_433:

time      : 2024-01-15 17:31:18
model     : Wireless-MBus Mode      : T            Manufacturer: AXI         ID        : 6422829
Version   : 1            Device Type: 0x07         Device Type String: Water Control   : 0x44          Data Length: 68           Data      : 414409072928420601078c20ee900f002c25eec00200069ffcec786fb1457aee00200710b591d37f2f427b0a2e2bbe28fba392c282f4785436694fecc0a247193eb490e8fe1d
Integrity : CRC
Control Info: 0x8C       Access number: 0x00       Device Type: 0x00         Configuration Word: 0x0000  Volume flow[1]: 2688.777 m3/min                   Min Volume flow[0]: 0.000 mm3/min

Data from wmbusmeters, with a IM871A dongle:

(meter) created MyTapWater q400 06422829 encrypted
(meter) used meter template MyTapWater 06422829 q400 to match 06422829
(meter) started meter 1 (MyTapWater 06422829 q400)
(meter) MyTapWater(1) q400  handling telegram from 06422829
(telegram) DLL L=43 C=44 (from meter SND_NR) M=0709 (AXI) A=06422829 VER=01 TYPE=07 (Water meter) (driver q400) DEV=im871a[xxxxxxxx] RSSI=-55
(telegram) ELL CI=8c CC=20 (slow_resp sync) ACC=ee
(telegram) AFL CI=90
(telegram) TPL CI=7a ACC=ee STS=00 CFG=0720 (AES_CBC_NO_IV)
telegram=|434409072928420601078C20EE900F002C25EEC00200069FFCEC786FB1457AEE002007102F2F_046D21110F3104136A3E0000446D000001314413C43800002F2F2F2F2F2F|+17
(wmbus) telegram from 06461940 ignored by all configured meters!

And some extra notes.

  • Why is only m_bus_output_data() adjusting the length?
  • Why the asymmetry? data[0] decreased and length increased?
  • Why is it necessary to "patch" the data, and not only the length field?
  • All data was received from the air, so why mess around with it at all.
  • If you would like the put the CRC at the end into the output, should it not be sufficient to increase length alone?

@merbanan
Copy link
Owner

The reason for the telegram output is to match the IM871A output. That should be adjusting the length data and removing the crc bytes.

If the current output does not match this then we have bugs in the codec and fixes are welcome.

@gdt
Copy link
Collaborator

gdt commented Aug 4, 2024

Where are we on this?

@zuckschwerdt
Copy link
Collaborator

We decided in #2711 that this can be applied. Further cleanup will be done in #2997.

@zuckschwerdt zuckschwerdt merged commit 6f7846e into merbanan:master Aug 4, 2024
8 checks passed
zuckschwerdt added a commit that referenced this pull request Nov 13, 2024
Include the final CRC for wmbusmeters to verify decryption.
Partially rolls back #2749 change.
zuckschwerdt added a commit that referenced this pull request Nov 13, 2024
Include the final CRC for wmbusmeters to verify decryption.
Partially rolls back #2749 change.
@zuckschwerdt
Copy link
Collaborator

For posterity: as suspected this change broke the Mode C Format B output for wmbusmeters as seen in #3089 and fixed with #3091

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.

4 participants