Skip to content

Map out data notifications in the rust api #6293

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.

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
greaka opened this issue Jan 1, 2025 · 2 comments · May be fixed by #6543
Open

Map out data notifications in the rust api #6293

greaka opened this issue Jan 1, 2025 · 2 comments · May be fixed by #6543
Assignees
Labels
Component: Rust API Issue needs changes to the Rust API Effort: Trivial Issue should take < 1 day Impact: Medium Issue is impactful with a bad, or no, workaround
Milestone

Comments

@greaka
Copy link

greaka commented Jan 1, 2025

What is the feature you'd like to have?
BNBinaryDataNotification would allow me to subscribe to data events emitted by BVs.

Is your feature request related to a problem?
I would like to write plugins using that api.

Are any alternative solutions acceptable?
Lot's of ideas on how to implement it, but the quick and easy one is probably preferable at this time.

Additional Information:
A quick and easy solution would be to expose the notifier as a trait and on callback init/register check for fn ptr equality for impls compared to a dummy struct to only subscribe to what's implemented.

@emesare emesare self-assigned this Jan 1, 2025
@emesare emesare added Effort: Low Issue should take < 1 week Impact: Medium Issue is impactful with a bad, or no, workaround Effort: Trivial Issue should take < 1 day Component: Rust API Issue needs changes to the Rust API labels Jan 1, 2025
@emesare emesare added this to the Gallifrey milestone Jan 1, 2025
@emesare emesare removed the Effort: Low Issue should take < 1 week label Jan 1, 2025
@plafosse plafosse modified the milestones: Gallifrey, H Feb 10, 2025
@emesare
Copy link
Member

emesare commented Mar 13, 2025

For when this gets tackled we want to make sure that we can pass nullptrs to notification callbacks we are not registering, otherwise we will end up jumping to and from the core frequently, this is not the end of the world but here especially is crucial.

For context a lot of these callback structs are opt-in so passing nullptr to the field onMySymbolRegistered or whatever will mean that you are not interested in receiving that and the core will not add that to the list of functions to call.

@rbran
Copy link
Contributor

rbran commented Apr 1, 2025

The PR #6543 avoid the unnecessary jumping from core to rust if unimplemented, but at a cost:

For the custom implementation, it's more complex and require some boiler plate on the user part, this can be reduced once the feature associated_type_defaults is stabilize.

For the closure implementation, the call require one extra indirection due to the dyn FnMut.

@emesare emesare linked a pull request Apr 17, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Rust API Issue needs changes to the Rust API Effort: Trivial Issue should take < 1 day Impact: Medium Issue is impactful with a bad, or no, workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants