Skip to content
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

pycsp: Implement Python binding for csp_sfp_send #15

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

jichgom
Copy link

@jichgom jichgom commented Jul 1, 2024

The function sets internal flags on csp_conn_t that are not accessible in Python, making it hard to implement in Python.

There is also a commit included in this that implements freeing of Py_Buffer in the bindings. Have a look at the commit message for more details

@jichgom jichgom requested a review from mmeisner July 1, 2024 06:49
Copy link

@mmeisner mmeisner left a comment

Choose a reason for hiding this comment

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

nice bugfixes!

src/bindings/python/pycsp.c Outdated Show resolved Hide resolved
src/bindings/python/pycsp.c Outdated Show resolved Hide resolved
@mmeisner mmeisner self-requested a review September 3, 2024 01:55
The documentation for `w*` specifies that
> The caller have to call PyBuffer_Release() when it is done with the buffer.

https://docs.python.org/3/c-api/arg.html
The `csp_sfp_send()` function is missing from the Python interface.
Furthermore, it is impossible to otherwise implement in pure Python
using the existing `pycsp` functions because: The `csp_sfp_send()`
function sets `CSP_FFRAG` flag in each packet header before
`csp_send()`. That flag cannot be set using`pycsp_connect` so it is in
effect an internal flag.
@mmeisner
Copy link

mmeisner commented Sep 3, 2024

@Wramberg, hvad er det nu for en pseudo person product-sw? man skal sætte på review?
Completion kommer ikke op med noget?

@Wramberg Wramberg requested a review from a team September 4, 2024 05:43
@Wramberg
Copy link

Wramberg commented Sep 4, 2024

Teamet skal stå som collaborator på repo ellers kan man ikke vælge det som reviewer

@mmeisner mmeisner removed the request for review from Wramberg September 4, 2024 06:03
@mmeisner
Copy link

@Wramberg, will anyone from products review this?
We also need a release afterwards...

@larseje
Copy link

larseje commented Sep 12, 2024

I recently added bindings to csp_recvfrom and buffree:
https://github.com/GomSpace/libgscsp/commit/0f0d49e7aeb73b6295d938e09bc53c1471e82fc5
https://github.com/GomSpace/libgscsp/commit/4dfe81613a8dcdd0666dc2ed10d65b4e16edbbd8

I added those binding to libgscsp to avoid having this intermediate libcsp fork with minimal changes compared to upstream.

I know we have stuff in GomSpace/libcsp already, but just wanted to avoid keep feeling up the fork. (I'd rather see it gone, with libgscsp only wrapping libcsp/libcsp).

Any chance the new binding here can be implemented in libgscsp instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants