Skip to content

[WIP] Add CSV parser#234

Closed
patrickelectric wants to merge 5 commits into
masterfrom
csv
Closed

[WIP] Add CSV parser#234
patrickelectric wants to merge 5 commits into
masterfrom
csv

Conversation

@patrickelectric

@patrickelectric patrickelectric commented Jul 30, 2018

Copy link
Copy Markdown
Member

Fix #131
Needs #61

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
@patrickelectric patrickelectric requested a review from jaxxzer July 30, 2018 21:58
@patrickelectric patrickelectric changed the title Add CSV parser [WIP] Add CSV parser Aug 2, 2018
@patrickelectric

Copy link
Copy Markdown
Member Author

Needs @brianhBR approval.

@patrickelectric patrickelectric changed the title [WIP] Add CSV parser Add CSV parser Aug 20, 2018
Comment thread src/sensor/ping.cpp
//Check for valid checksum and id
ping_msg_ping1D_empty ping_message{(uint8_t*)(packages[i].data()), (uint16_t)packages[i].length()};
if(!ping_message.verifyChecksum()) {
continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should warn here.

@jaxxzer jaxxzer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need timestamps in the csv.

@jaxxzer

jaxxzer commented Aug 20, 2018

Copy link
Copy Markdown
Contributor

I'm wondering what's the most useful thing here. For example, if we receive distance info from two different messages, the data lands in two different files. Perhaps we should stick to only pertinent information (confidence, range, profile) and interleave this data from all messages into one output file. Meanwhile, we can forgo exporting things like firmware version, voltage, scan range etc. This consideration should not hold up this PR, however.

@jaxxzer jaxxzer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are all the function mappings returning floats?
Is it because the functions in the map must all have the same type?

@patrickelectric

patrickelectric commented Aug 21, 2018

Copy link
Copy Markdown
Member Author

I'm wondering what's the most useful thing here. For example, if we receive distance info from two different messages, the data lands in two different files. Perhaps we should stick to only pertinent information (confidence, range, profile) and interleave this data from all messages into one output file. Meanwhile, we can forgo exporting things like firmware version, voltage, scan range etc. This consideration should not hold up this PR, however.

I would like to add each variable in a different tab in excel, and in the end only one file with a bunch of tabs. But this will need second party libraries or new classes to handle everything. This is a simple and good first version.

Why are all the function mappings returning floats?
Is it because the functions in the map must all have the same type?

Exactly:
https://github.com/bluerobotics/ping-protocol-cpp/pull/61/files#diff-ebcbbb8c54e652539da8ea1febd4bf6eR34
All values are number of now, and I don't see an objective to save text in csvs files for now.

@jaxxzer

jaxxzer commented Aug 27, 2018

Copy link
Copy Markdown
Contributor

Add timestamps or open an issue for it after merge.

@patrickelectric patrickelectric changed the title Add CSV parser [WIP] Add CSV parser Dec 17, 2018
@patrickelectric

Copy link
Copy Markdown
Member Author

Closing due to inactivity.

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