-
Notifications
You must be signed in to change notification settings - Fork 49
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
make DZG workaround more specific #136
Conversation
can't test, but LGTM! i'd leave you some time to address my comments (if you like), then i'd merge. (we really want units-tests for this kind of stuff, but that's beyond the scope of this fix.) |
I pushed but github seems to think about it for a long time |
i haven't the slightest idea why github is refusing to run the workflow... |
It did eventually (after about an hour) show that I force-pushed, so it should be updated now. |
there's still no pipeline run and no option for me to start one 😕 |
In volkszaehler#132 and volkszaehler/vzlogger#533 it was reported that there are DZG meters that fall into the range currently deemed broken (all DZG meters with serial numbers before 6000 0000), but aren't actually broken. The reason for this seemed to be that they're a different type than mine that was broken. Further inquiry with DZG revealed that indeed only DVS74 meters are affected by the encoding issue, and then _those_ only for serial numbers before 6000 0000. They also said that (currently) the serial number ranges - 4200 0000 to 4899 9999, and - 5500 0000 to 5899 9999 are reserved for DVS74 meters. As such, make the workaround detection more specific, and we no longer need to have the check for 6000 0000 either, just need to have a list of affected ranges. To achieve that, refactor the code into a separate function that actually decodes the serial number to an integer, so we can make the comparison more easily and have the list of affected numbers more clearly in the code. This should fix volkszaehler#132 since the serial number there starts with 4005, so it's clearly not in the affected ranges, as explained by my contact at DZG. For volkszaehler/vzlogger#533 I don't have the serial number, but it's the same type of meter as in volkszaehler#132.
Looks better now, perhaps? I pushed a new version w/o any changes. |
https://github.com/volkszaehler/libsml/pull/136/checks
🎉 |
In #132 and volkszaehler/vzlogger#533 it was reported that there are DZG meters that fall into the range currently deemed broken (all DZG meters with serial numbers before 6000 0000), but aren't actually broken. The reason for this seemed to be that they're a different type than mine that was broken.
Further inquiry with DZG revealed that indeed only DVS74 meters are affected by the encoding issue, and then those only for serial numbers before 6000 0000. They also said that (currently) the serial number ranges
As such, make the workaround detection more specific, and we no longer need to have the check for 6000 0000 either, just need to have a list of affected ranges. To achieve that, refactor the code into a separate function that actually decodes the serial number to an integer, so we can make the comparison more easily and have the list of affected numbers more clearly in the code.
This should fix #132 since the serial number there starts with 4005, so it's clearly not in the affected ranges, as explained by my contact at DZG. For volkszaehler/vzlogger#533 I don't have the serial number, but it's the same type of meter as in #132.