dstroy0 / InputHandler

Arduino input handler
https://dstroy0.github.io/InputHandler/
GNU General Public License v3.0
1 stars 0 forks source link

delimiters could be a sequence of bytes #12

Closed 2bndy5 closed 2 years ago

2bndy5 commented 2 years ago

I came across this https://github.com/dstroy0/InputHandler/blob/e814bfcf3941218ca069253f33793c734d58d41a/InputHandler.cpp#L85-L87 and thought that delimiters shouldn't be limited to a single byte.

My initial reaction was to instead use

memcmp((char*)_delim_, (char*)incoming, strlen(_delim_)) == 0

but that would require incoming be a ptr to the position in the data buffer.

I also don't fully understand the iscntrl(incoming) check here. Is it supposed to ignore a premature EoL?


Sorry if it seems like I'm critically dissecting the lib here. I'm not trying to be offensive, rather I'm just trying to understand some design decisions.

dstroy0 commented 2 years ago

No it’s cool, don’t worry about it. The code works so making it better is the goal now, gotta ask questions about why this is that way or could it be better another way, it’s an important part of the process. Delimiters do not have to be limited to a single byte, the comparison can easily be changed to const char*. You know what I was thinking about doing is making parse functions for different use cases like NMEA sentences that start with $ that the user can drop in. We can keep those in a folder called snippets or something in examples.

If you notice the position of the iscntrl() it is looking for hanging control characters outside of arguments.

On Mar 4, 2022, at 3:58 PM, Brendan @.***> wrote:

 I came across this https://github.com/dstroy0/InputHandler/blob/e814bfcf3941218ca069253f33793c734d58d41a/InputHandler.cpp#L85-L87 and thought that delimiters shouldn't be limited to a single byte.

My initial reaction was to instead use

memcmp((char)delim, (char)incoming, strlen(delim)) == 0 but that would require incoming be a ptr to the position in the data buffer.

I also don't fully understand the iscntrl(incoming) check here. Is it supposed to ignore a premature EoL?

Sorry if it seems like I'm critically dissecting the lib here. I'm not trying to be offensive, rather I'm just trying to understand some design decisions.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

2bndy5 commented 2 years ago

You know what I was thinking about doing is making parse functions for different use cases like NMEA sentences that start with $ that the user can drop in.

This idea could be shipped with the lib src under a folder called "InputHandlerHelpers". Before we start adding to the file structure, I think it would be best to

You can also have a look at the Arduino Library specs. The main idea is to avoid cluttering the repo root.

If you ever decide to patch this lib for PC support, I can implement the build system using CMake (best for cross-platform building/installing).

dstroy0 commented 2 years ago

Delimiter is a const char*, can be set to anything. The library structure has been fixed.