Skip to content

Fix InterfaceVillasQueueless signal indexing, read handling, and start-of-sim sync #585

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.

Sign up for GitHub

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

Open
leonardocarreras wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix InterfaceVillasQueueless signal indexing, read handling, and start-of-sim sync #585

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 28 additions & 15 deletions dpsim-villas/src/InterfaceVillasQueueless.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ void InterfaceVillasQueueless::createSignals() {
}
nodeOutputSignals->push_back(std::make_shared<node::Signal>(
"", "", stdTypeToNodeType(attr->getType())));
idx++;
}

node::SignalList::Ptr nodeInputSignals = mNode->getInputSignals(true);
Expand Down Expand Up @@ -218,20 +219,19 @@ Int InterfaceVillasQueueless::readFromVillas() {
}
try {
sample = node::sample_alloc(&mSamplePool);
ret = 0;
while (ret == 0) {
ret = mNode->read(&sample, 1);
if (ret < 0) {
SPDLOG_LOGGER_ERROR(mLog,
"Fatal error: failed to read sample from "
"InterfaceVillas. Read returned code {}",
ret);
close();
std::exit(1);
} else if (ret == 0) {
SPDLOG_LOGGER_WARN(mLog,
"InterfaceVillas read returned 0. Retrying...");
}
if (sample == nullptr) {

@pipeacosta pipeacosta Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I interpret this correctly, this change implies that the InterfaceVillas tries only once, what's the reason for this?

@leonardocarreras leonardocarreras Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pipeacosta! Thanks for the question, is true that is a change on the way we do things. My way of thinking is that queued interface will work in a dedicated thread, so it can retry. And for the queueless, we read in the PreStep, so the simulation will block until we get data in the existing implementation.
I was doing some experiments with the RT mode on DPsim. We gain robustness in some edge cases, reusing the last sample if we have one miss... For some RT experiments, it blocks too long for one single de-sync, or it exits with error... So basically one best effort in reading, if nothing is there, we continue with an old sample instead of exiting with error... It might clash with some behaviour that depends on "new sample arrives, then we run one step", so sometimes will reuse an old sample for the external pacing source...
An intermediate solution would be making this behaviour as selectable option, would that be better in your opinion? We can use the blockOnRead argument for that (this is not implemented in that way currently) so is possible to have either of them

@pipeacosta pipeacosta Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. I would stay with the philosophy of trying to keep real-time execution with the queueless interface, and use previous samples if needed. It would be good to log a warning in this case. Having said that, it is not necessary to implement the intermediate solution you mentioned imo.

SPDLOG_LOGGER_ERROR(mLog, "InterfaceVillas could not allocate a sample!");
return mSequenceToDpsim;
}

ret = mNode->read(&sample, 1);
if (ret <= 0) {
if (ret < 0)
SPDLOG_LOGGER_ERROR(mLog, "InterfaceVillas read failed: {}", ret);
else
SPDLOG_LOGGER_WARN(mLog, "InterfaceVillas read returned 0.");
sample_decref(sample);
return mSequenceToDpsim;
}

if (sample->length != mImportAttrsDpsim.size()) {
Expand Down Expand Up @@ -270,6 +270,9 @@ Int InterfaceVillasQueueless::readFromVillas() {
}
}

if (sample->flags & (int)villas::node::SampleFlags::HAS_SEQUENCE)
seqnum = sample->sequence;

sample_decref(sample);
} catch (const std::exception &) {
if (sample)
Expand Down Expand Up @@ -355,7 +358,17 @@ void InterfaceVillasQueueless::writeToVillas() {
}

void InterfaceVillasQueueless::syncImports() {
// Block on read until all attributes with syncOnSimulationStart are read
// Only block if some import is marked to sync at simulation start.
bool needSync = false;
for (const auto &attr : mImportAttrsDpsim) {
if (std::get<3>(attr)) {
needSync = true;
break;
}
}
if (!needSync)
return;

mSequenceToDpsim = this->readFromVillas();
}

Expand Down
Loading