Use inp/out links for mapping terminals #44
Conversation
This should help avoid mistakes in branchy return code
The previous design pattern didn't make much sense
|
Going to mark this as ready now. Once approved (or mostly approved) I'll regenerate the templates |
ek9000App/src/devEK9000.cpp
Outdated
| : drvModbusAsyn(portName, octetPortName, 0, 2, -1, 256, dataTypeUInt16, 150, "") { | ||
|
|
||
| /* Initialize members */ | ||
| m_terms = static_cast<devEK9000Terminal*>(malloc(sizeof(devEK9000Terminal) * termCount)); |
There was a problem hiding this comment.
For my own information, why the mix of malloc/free and not just use new[] operator to allocate all terminals?
There was a problem hiding this comment.
I was confused at my own code here too, but I just remembered: new[] requires a default constructible type, which devEK9000Terminal is not because its constructor takes a parameter.
There was a problem hiding this comment.
I'll probably make m_terms a std::vector<devEK9000Terminal*> instead- just realized that the destructor is not being called on any devEK9000Terminals because the array is simply being freed.
There was a problem hiding this comment.
Ah, I see. I like the std::vector approach much better 👍
Also clamp num terminals to <= 0xFF in ek9000Register
|
I'll do that right now! |
|
Yep, seeing this in database form reinforces my opinion that it's a good change 👍 |
This ended up being a sizable refactor, but ultimately fixes the biggest issue with the module. Previously record -> terminal mapping was determined by
ek9000ConfigureTerminaland a numeric suffix on the record name. Now it's done entirely using an inp/out link in theINST_IOformat, similar to how asyn does it.The legacy logic still exists in the module, not sure if I should keep it around or axe it entirely though.
Some other changes made here:
devEK9000to derive directly fromdrvModbusAsyn, as it should've in the first placestrdupcalls I missed during an earlier refactorStill a draft for now until I decide what to do with the legacy paths. Also haven't regenerated the DB templates yet, just to make it easier to review :)