Don't bool-test explicit datastores#2638
Conversation
Empty data stores tend to be false-y when tested, which is not the intent here.
|
What do you mean "backported to master" ? we never work on master except when merging for a release, so this PR like other PRs will be part of the next release. If you refer to making a new x.y.Z release of an old version, that is something we do not do, we only support v3.9.x. When v4.0.0 is released we will continue to support v3.9.x for 1-2 years allowing plenty of time to upgrade. |
| self.store["d"] = di if di is not None else ModbusSequentialDataBlock.create() | ||
| self.store["c"] = co if di is not None else ModbusSequentialDataBlock.create() | ||
| self.store["i"] = ir if di is not None else ModbusSequentialDataBlock.create() | ||
| self.store["h"] = hr if di is not None else ModbusSequentialDataBlock.create() |
There was a problem hiding this comment.
The last 3 lines of the new code look like cut'n'paste mistakes.
e.g. the original code
self.store["c"] = co or ModbusSequentialDataBlock.create()
tested variable co and conditionally assigned it
in the new code, co is conditionally assigned but di was tested:
self.store["c"] = co if di is not None else ModbusSequentialDataBlock.create()
There was a problem hiding this comment.
Ugh. Obviously. Thanks, will fix ASAP.
There was a problem hiding this comment.
@smurfix I thought you had tested the code, if you open a new PR please add tests to ensure it works correctly.
But you have time, because unless a serious bug is found there will be no 3.9 updates before late summer.
|
Yeah I did test the code, but unfortunately I did it with |
3 participants
Empty data stores tend to be false-y when tested,
which is not the intent here.
Should probably be backported to master