Conversation
zerobuf/Zerobuf.cpp
Outdated
| rhs.getZerobufNumDynamics( ))); | ||
| uint32_t& version = rhs._allocator->getItem< uint32_t >( 0 ); | ||
| version = ZEROBUF_VERSION_ABI; | ||
| rhs.notifyUpdated(); |
There was a problem hiding this comment.
not sure about that. The new values will be undefined, so I'm not sure how useful it is to signal the change. Users should stop using a moved object anyway.
There was a problem hiding this comment.
Right, this is one of the questions I have about the code (I just moved down the notify call). That's why I had not assigned anyone yet...
There was a problem hiding this comment.
This is indeed strange. I would expect (this->)notifyChanging()...
There was a problem hiding this comment.
Agree, it should be removed. For the constructor (this->)notifyChanging() has no effect in any case because nothing can be connected to it already.
There was a problem hiding this comment.
Yes, I removed it. This PR has been blocked for now (trying to find a general solution in HBPVIS/Servus#53)
|
Updated and Changed (what Changing should have been initially) are potentially two different things. From the way we used it today: Updated happens after a 'remote' change (subscriber, http server, load from file) on the object, where as Changed on every set/modification on the local object instance. This Changed state is vital for a 'dirty' tracking (c.f. Collage, save to file, ...), which could potentially lead to an endless (re-)distribution between two applications. |
|
@tribal-tec yes exactly. Updated should only be emitted for remote changes, probably by the servus::Serializable direcly in the _fromJSON and _fromBinary, and the function can be made private then. |
jafyvilla
force-pushed
the
notify_update
branch
from
5ea1527 to
90c3be0
Compare
| notifyChanging(); | ||
| _allocator->copyBuffer( rhs._allocator->getData(), | ||
| rhs._allocator->getSize( )); | ||
| notifyChanged(); |
There was a problem hiding this comment.
ack, but this is the assignment operator.
There was a problem hiding this comment.
oops my bad, github had collapsed the two functions..!
jafyvilla
force-pushed
the
notify_update
branch
from
90c3be0 to
34562a0
Compare
bin/zerobufCxx.py
Outdated
| return "uint32_t" if self.is_zerobuf_type or self.is_enum_type else self.type | ||
|
|
||
|
|
||
| """Doxygen documentation for C++ functions""" |
There was a problem hiding this comment.
Move below class declaration. Python docstrings are after the declaration.
jafyvilla
force-pushed
the
notify_update
branch
from
34562a0 to
55e1a7a
Compare
|
+1 |
Notify changes by calling Servus::serializable::notifyChanged() after the object has been changed (and not before). Generate doxygen documentation automatically for setters and reference getters.
jafyvilla
force-pushed
the
notify_update
branch
from
55e1a7a to
4408665
Compare
4 participants
Notify changes by calling notifyUpdated()
after the object has been changed (and not before).
Remove notifyChanging().
Generate doxygen documentation automatically for setters and
reference getters.