Fix: Prepend simtime to ZMQ write() payload for consistency (Issue #244)#258
Merged
pradeeban merged 1 commit intoFeb 11, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes inconsistency between file-based and ZMQ-based concore.write() by adding a simtime prefix to ZMQ list payloads, addressing Issue #244 where mixed ZMQ/file studies could misinterpret the first data element as simtime.
Changes:
- Prepend
[simtime + delta]to ZMQ list payloads inconcore.write()to match file write behavior. - Update the ZMQ write unit test to expect the simtime-prefixed payload.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| concore.py | Adjusts ZMQ write() to prefix simtime for list payloads (aligning with file output format). |
| tests/test_concore.py | Updates test expectations to include simtime prefix in the ZMQ send payload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ntrolCore-Project#244) - ZMQ write now prepends [simtime + delta] to match file-based write behavior - ZMQ read now strips simtime prefix (mirroring file-based read behavior) - Updated docstring to clarify val is data-only - Added round-trip test to verify write+read returns original data
a8009fa to
0e49e15
Compare
Member
|
/gemini review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey pradeeban,
Fixes #244.
The
write()function behaved differently for ZMQ and file-based communication.In the file-based path,
[simtime + delta]is prepended before writing the data.But in the ZMQ path, the value was sent directly without the simtime prefix.
Since
read()always expects the first element (inval[0]) to be simtime, this caused problems in mixed setups where one component uses ZMQ and another uses file-based communication. In those cases the first data value could be interpreted as simtime, resulting in incorrect data and one fewer value being returned.To fix this, the ZMQ write path now prepends
simtime + deltato list payloads so that it behaves the same as the file-based implementation.Updated logic:
Changes in this PR:
concore.py– prepend simtime to ZMQ list payloads inwrite()test_concore.py– updated test to expect the simtime prefixTesting: