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

HW Monitor - Terminal Parser API #6623

Merged

Conversation

remibettan
Copy link
Contributor

Adding terminal parser capability to API.
Auto complete not implemented here.
Triggered by jira ticket: DSO-14959

common/parser.hpp Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
src/rs.cpp Show resolved Hide resolved

rs2_raw_data_buffer* rs2_terminal_parse_response(rs2_terminal_parser* terminal_parser,
const char* command, unsigned int size_of_command,
const void* response, unsigned int size_of_response, rs2_error** error) BEGIN_API_CALL
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add validation for size_of_cmd/response

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done - hope the numbers are ok

src/terminal-parser.cpp Outdated Show resolved Hide resolved
src/terminal-parser.h Outdated Show resolved Hide resolved
src/terminal-parser.h Show resolved Hide resolved
src/terminal-parser.h Outdated Show resolved Hide resolved
@ev-mp ev-mp changed the title Terminal parser only add to api HW Monitor - Terminal Parser API Jun 17, 2020
@@ -3,12 +3,12 @@

#include <librealsense2/rs.hpp> // Include RealSense Cross Platform API
#include "example.hpp" // Include short list of convenience functions for rendering

#include <fstream>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is not part of the PR

src/rs.cpp Outdated
@@ -2988,6 +2988,7 @@ rs2_raw_data_buffer* rs2_terminal_parse_command(rs2_terminal_parser* terminal_pa
{
VALIDATE_NOT_NULL(terminal_parser);
VALIDATE_NOT_NULL(command);
VALIDATE_LE(size_of_command, 1000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment for the hard-coded values - units/explanation

@remibettan remibettan force-pushed the terminal-parser-only-add-to-api branch from b7a8172 to 70829bc Compare June 17, 2020 13:11
Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

Couple of typos in the new code - but these seem the last onces

src/rs.cpp Outdated
{
VALIDATE_NOT_NULL(terminal_parser);
VALIDATE_NOT_NULL(command);
VALIDATE_LE(size_of_command, 1000);//bufer shall be lesss than 1000 kbytes or similar
Copy link
Collaborator

Choose a reason for hiding this comment

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

bytes, correct?

src/rs.cpp Outdated
VALIDATE_NOT_NULL(terminal_parser);
VALIDATE_NOT_NULL(command);
VALIDATE_NOT_NULL(response);
VALIDATE_LE(size_of_command, 1000); //bufer shall be lesss than 1000 kbytes or similar
Copy link
Collaborator

Choose a reason for hiding this comment

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

lesss, .. bytes - appears in several places

@remibettan remibettan force-pushed the terminal-parser-only-add-to-api branch from bed1cc8 to 1937c3c Compare June 17, 2020 14:18
Copy link
Collaborator

@ev-mp ev-mp left a comment

Choose a reason for hiding this comment

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

LGTM

@ev-mp ev-mp merged commit 082da49 into IntelRealSense:development Jun 17, 2020
@remibettan remibettan deleted the terminal-parser-only-add-to-api branch March 15, 2021 06:54
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