Skip to content

Port kafka node-type to C++ and simplify it - #986

Open
stv0g wants to merge 1 commit into
masterfrom
node-kafka-cpp
Open

Port kafka node-type to C++ and simplify it#986
stv0g wants to merge 1 commit into
masterfrom
node-kafka-cpp

Conversation

@stv0g

@stv0gstv0g commented Nov 27, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@stv0g
stv0g requested a review from n-eiling as a code ownerNovember 27, 2025 14:27
@al3xa23
al3xa23 requested a review from pipeacostaJuly 29, 2026 07:22
@stv0gstv0g changed the title feat(node-kafka): Port kafka node-type to C++ and simplify itPort kafka node-type to C++ and simplify itJul 29, 2026
@stv0gstv0g added enhancement New feature or request node::kafka labels Jul 29, 2026
@al3xa23al3xa23 linked an issue Jul 29, 2026 that may be closed by this pull request
Port Kafka node-type to C++ #759
Open
@steffen-vogel-opalrt
feat(node-kafka): Port kafka node-type to C++ and simplify it
fdfdbae
Signed-off-by: Steffen Vogel <steffen.vogel@opal-rt.com>
al3xa23
al3xa23 reviewed Aug 13, 2026
View reviewed changes

@al3xa23al3xa23 left a comment

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.

Tested the config, too. The only thing is that no values are displayed when receiving in kafka. Is that intended?

Image

Comment threadlib/nodes/kafka_old.cpp

@al3xa23al3xa23Aug 13, 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.

Do you want to keep the kafka_old.cpp as backup?
If we keep it, how shall we proceed with deleting the header file?

Comment threadlib/nodes/kafka.cpp
* Author: Juan Pablo Noreña <jpnorenam@unal.edu.co>
* Author: Steffen Vogel <steffen.vogel@opal-rt.com>
* SPDX-FileCopyrightText: 2021 Universidad Nacional de Colombia
* SPDX-FileCopyrightText: 2025 OPAL-RT Germany GmbH

@al3xa23al3xa23Aug 13, 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.

2026?

@pipeacosta

pipeacosta commented Aug 14, 2026•
edited
Loading

Copy link
Copy Markdown

Tested the config, too. The only thing is that no values are displayed when receiving in kafka. Is that intended?
Image

I tested the config as well. I verified the received samples with the kafka consumer, as explained here:

./kafka-console-consumer.sh --bootstrap-server localhost:9092 --topic test-topic --from-beginning

Here some of the samples:

[{"schema": {"type": "struct", "name": "villas-node.Value", "fields": [{"type": "int64", "optional": false, "field": "timestamp"}, {"type": "int64", "optional": false, "field": "sequence"}]}, "payload": {"timestamp": 1786700627162, "sequence": 3113, "random": -5.039474206528018, "sine": -0.80993710519715645, "square": 1.0, "triangle": -0.39900236799996947, "ramp": 0.65024940800000763}}] [{"schema": {"type": "struct", "name": "villas-node.Value", "fields": [{"type": "int64", "optional": false, "field": "timestamp"}, {"type": "int64", "optional": false, "field": "sequence"}]}, "payload": {"timestamp": 1786700627212, "sequence": 3114, "random": -4.9923175974169647, "sine": -0.95160353643197448, "square": 1.0, "triangle": -0.19886997200001133, "ramp": 0.70028250699999717}}] [{"schema": {"type": "struct", "name": "villas-node.Value", "fields": [{"type": "int64", "optional": false, "field": "timestamp"}, {"type": "int64", "optional": false, "field": "sequence"}]}, "payload": {"timestamp": 1786700627262, "sequence": 3115, "random": -4.6567145497036391, "sine": -0.99999855596582099, "square": 1.0, "triangle": 0.0010818920000019716, "ramp": 0.75027047300000049}}] [{"schema": {"type": "struct", "name": "villas-node.Value", "fields": [{"type": "int64", "optional": false, "field": "timestamp"}, {"type": "int64", "optional": false, "field": "sequence"}]}, "payload": {"timestamp": 1786700627312, "sequence": 3116, "random": -4.695877600033139, "sine": -0.95057359456454704, "square": 1.0, "triangle": 0.200992508000013, "ramp": 0.80024812700000325}}] [{"schema": {"type": "struct", "name": "villas-node.Value", "fields": [{"type": "int64", "optional": false, "field": "timestamp"}, {"type": "int64", "optional": false, "field": "sequence"}]}, "payload": {"timestamp": 1786700627362, "sequence": 3117, "random": -4.6060945505198241, "sine": -0.8081307148190634, "square": 1.0, "triangle": 0.40095891999999367, "ramp": 0.85023972999999842}}]

For the test, I prepared a docker compose file, wouldn't it be beneficial to include it in the examples folder?

pipeacosta
pipeacosta reviewed Aug 14, 2026
View reviewed changes

@pipeacostapipeacosta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the PR @steffen-vogel-opalrt, sorry I didn't have much time to look into it in more detail, I left a couple of comments.

Comment threadlib/hooks/lua.cpp Outdated
if (ret)
throw ConfigError(json_sigs, "node-config-hook-lua-signals",
"Setting 'signals' must be a list of dicts");
signalsExpressions->parse(json_sigs);

@pipeacostapipeacostaJul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you explain why you removed the throw part? are we catching the exception somewhere else now?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pipeacostapipeacostapipeacosta left review comments
@al3xa23al3xa23al3xa23 left review comments
@n-eilingn-eilingAwaiting requested review from n-eilingn-eiling is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

enhancementNew feature or requestnode::kafka

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Port Kafka node-type to C++

4 participants

@stv0g@pipeacosta@al3xa23@steffen-vogel-opalrt