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

Add overloads for GetBuffer(..) and SetBuffer(..) that take an explicit offset and length. #394

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

timyhac
Copy link
Collaborator

@timyhac timyhac commented Jul 10, 2024

No description provided.

@timyhac timyhac merged commit c18df56 into master Jul 11, 2024
1 check passed
@timyhac timyhac deleted the #393 branch July 11, 2024 03:39
@MitchRazga
Copy link

Would be beneficial if there were (ReadOnly)Span<byte> overloads to reduce allocations and make using existing buffers easier.
I'm happy to do a PR to implement if you are interested.

The native method declarations in C# would need to change since they use byte[] but won't be too much trouble since the native API uses a pointer anyway.

[DllImport(DLL_NAME, EntryPoint = nameof(plc_tag_get_raw_bytes), CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)]
public static extern int plc_tag_get_raw_bytes(Int32 tag_id, int start_offset, [Out] byte[] buffer, int buffer_length);
[DllImport(DLL_NAME, EntryPoint = nameof(plc_tag_set_raw_bytes), CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)]
public static extern int plc_tag_set_raw_bytes(Int32 tag_id, int start_offset, [In] byte[] buffer, int buffer_length);

https://github.com/libplctag/libplctag/blob/c55bc5876d938dda1c609750cde5ae4812d7b8a8/src/lib/libplctag.h#L505-L506

LIB_EXPORT int plc_tag_set_raw_bytes(int32_t id, int offset, uint8_t *buffer, int buffer_length);
LIB_EXPORT int plc_tag_get_raw_bytes(int32_t id, int offset, uint8_t *buffer, int buffer_length);

There are a few ways to do this, one approach can be found in Memory<T> and Span<T> usage guidelines under:

Rule #9: If you're wrapping a synchronous p/invoke method, your API should accept Span as a parameter.

Alternatively and ideally, would use P/Invoke source generation but this requires .NET 7+. Something like:

[LibraryImport(DLL_NAME, EntryPoint = nameof(plc_tag_get_raw_bytes))]
[UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])]
public static partial int plc_tag_get_raw_bytes(Int32 tag_id, int start_offset, [MarshalUsing(CountElementName = nameof(buffer_length))] out Span<byte> buffer, int buffer_length);

[LibraryImport(DLL_NAME, EntryPoint = nameof(plc_tag_set_raw_bytes))]
[UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])]
public static partial int plc_tag_set_raw_bytes(Int32 tag_id, int start_offset, ReadOnlySpan<byte> buffer, int buffer_length);

This would also make it possible to create ReadAsync and WriteAsync with Memory<byte> overloads too. Similar to Stream.ReadAsync and Stream.WriteAsync

public ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default);
public ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default);

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.

2 participants