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

Implemented read #1

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

Implemented read #1

wants to merge 21 commits into from

Conversation

aqiank
Copy link

@aqiank aqiank commented Jun 13, 2016

Just getting familiar with the source code :)

Signed-off-by: Jacky Boen aqiank@gmail.com


This change is Reviewable

Jacky Boen added 2 commits June 13, 2016 20:01
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
@aqiank aqiank changed the title code organization Implemented read Jun 21, 2016
@aqiank
Copy link
Author

aqiank commented Jun 21, 2016

Hi Andreas,

I've implemented the read() part of the SerialIndex. I pulled out the data processing code into IO.cpp and IO.hpp and added test folder where I can easily test the processor against some sample data generated from test/sample.txt into test/sample.h without having to have Arduino. You should be able to test them by just running make and then ./test in the test folder. You can check out my branch at https://github.com/aqiank/serialindex/tree/boen/Arduino/serialindex.

The implementation most likely doesn't completely match what you want but can be adjusted fairly easily. For example, keys right now requires all of the characters to be alphabets and arrays aren't bound-checked because it doesn't store user's array information right now as I wanted to do as much as possible before having to allocate new memory.

Also at the moment, it validates the input before actually evaluating and setting values which means it goes through the value part of the data twice. It prevents user from mistakenly set only part of an array but if that's okay then I could get rid of it so it only has to go through the value part of the data once.

I implemented the ability to set array values partially which I call a slice right now but that can be discussed further. Its syntax currently looks like: nums={0=123} for setting one value and nums={0..3=123} for setting multiple values.

There is no heap memory used during the data processing so the only heap memory allocation operations are inside the constructor.

The code could use some DRYing up as there are a lot of pretty much identical code structures but I just keep it that way for now until we've finalized on the specifications so it's easier to change in case there's some special cases that need to be handled.

Anyway, there's still some stuff to do like bounds-checking but I believe the concept and implementation is there.

Jacky Boen added 8 commits June 21, 2016 22:24
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
…me structure as the rest

Signed-off-by: Jacky Boen <aqiank@gmail.com>
Jacky Boen added 4 commits June 22, 2016 03:08
…ket right away

Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
@sojamo
Copy link
Owner

sojamo commented Jun 22, 2016

awesome jacky, lets meet soon to discuss details and then merge.

Jacky Boen added 7 commits June 22, 2016 12:15
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
Signed-off-by: Jacky Boen <aqiank@gmail.com>
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.

None yet

2 participants