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

Feature/socketio #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Feature/socketio #14

wants to merge 2 commits into from

Conversation

ponast
Copy link

@ponast ponast commented Mar 28, 2021

Nice ring buffer implementation! I like it and plan to use it with socket-IO in a stack. I had an issue with writing and reading sockets as they require continuous memory segments. So I implemented a sockets interface extension to the ring buffer that allows for efficient access to sockets from Ringbuffer without intermediate copying steps. The extension requires some additional methods within the class to get the length of continuous data in the data_buff member. The actual socket read and write functions are implemented outside class Ringbuffer. All added methods to Ringbuffer can be made protected (except print which is just a quick fix for testing) if the two external functions are made friends as they likely shouldn't be directly accessed from elsewhere. I haven't done that.

I'm not too good at atomics programming and there might be better methods to implement the socket interface which I'm not aware of. Anyway, this is a first shot and if you don't think this sockets API is too specialized and of good enough quality then I think it would be nice to have it integrated with Ringbuffer.

The test file shows how the interface is used. I have added dummy socket-IO functions with Posix interface for testing in a rudimentary test sequence.

@@ -63,6 +63,17 @@ namespace jnk0le
}

/*!
* \brief For debug purposes. Outputs data_buff array in full
*/
void print()
Copy link

@drewr95 drewr95 Aug 26, 2022

Choose a reason for hiding this comment

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

  1. This will produce a compiler error for files that include this header that do not include <iostream> before including this header
  2. Since this library is OS independent, embedded applications could use this, and <string> is notoriously large and not typically used, which is included by <iostream>

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