Skip to content

serial interface: add command parser for protocol - #82

Merged
dhebbeker merged 73 commits into
developfrom
feature/60-serial-interface-add-command-parser-for-protocol
Dec 17, 2023
Merged

serial interface: add command parser for protocol#82
dhebbeker merged 73 commits into
developfrom
feature/60-serial-interface-add-command-parser-for-protocol

Conversation

@dhebbeker

@dhebbekerdhebbeker commented Nov 15, 2023

Copy link
Copy Markdown
Member

Purpose

Create a command parser which is capable to interpret the commands from serial interface according to the defined protocol.

Resolves#60

Acceptance criteria

  • It is OK for this version to respond with dummy responses (format does not need to follow protocol)
  • All arguments of the commands shall be understood (test by echo the values to serial interface)
  • Dummy functions for the commands shall be called (test by echo a string on serial interface)

Todos

  • remove trim functionality as it is not used
  • add member function to Command which generates a string which is intended to specify the correct use of the command (command line format)
  • handle the case that the command line can not be correctly interpreted outside of Command by printing a message
  • refactoring: check which function shall be member functions
  • add documentation
  • test: add appropriate test cases (check for code coverage)
  • create dedicated namespace
  • add test case for string helper functions
  • add documentation for string helper functions
  • add documentation on how to use the command line interpreter

Changes

  • .github/workflows/platformio.yml: Added test execution to workflow.
  • lib/3rd_party_adapters/serial_port.cpp: Change line extraction to omit line ending characters and empty lines.
  • lib/application_business_rules/Protocol.*: This implements the protocol (here: dummies) by using the command line interpreter.
  • lib/application_business_rules/command_line_interpreter.hpp: Essentially provides Option and Command class templates which can be used to call a function with some arguments based on a command line.
  • lib/enterprise_business_rules/string_helpers.*: Add helper functions which can be used to operate on strings.
  • platformio.ini: Add configuration for unit tests.
  • src/main.cpp: Attach ProtocolHandler to serial port.
  • test/test_cli/test_cli.cpp: Unit test for command line interpreter.
  • test/test_serial_port/test_serial_port.cpp: Adjust to changed fetching of lines. The end of line character is omitted now.
  • test/test_string_helpers/test_string_helpers.cpp: Unit test for string helpers.

@dhebbekerdhebbeker linked an issue Nov 15, 2023 that may be closed by this pull request
@dhebbeker
dhebbekerforce-pushed the feature/60-serial-interface-add-command-parser-for-protocol branch from aa8496b to 33aaeecCompareNovember 16, 2023 11:16
@dhebbekerdhebbeker mentioned this pull request Nov 25, 2023
2 tasks
@dhebbeker
dhebbekerforce-pushed the feature/60-serial-interface-add-command-parser-for-protocol branch 2 times, most recently from 97d40bf to b20de1aCompareNovember 25, 2023 10:45

@dhebbekerdhebbeker left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Did a self-review: Some documentation is missing.

@dhebbekerdhebbeker left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Did self-review: OK

@dhebbeker

Copy link
Copy Markdown
MemberAuthor

Tested also with real device:

 begin program '/home/dhebbeker/pro/time-tracker/src/time-tracker.ino compiled at Nov 30 2023 21:50:08
---- Sent utf8 encoded message: "list\n" ----
this is a list: a, b, c, ...
---- Sent utf8 encoded message: "list\n" ----
this is a list: a, b, c, ...
---- Sent utf8 encoded message: "edit --name \"hello world\"\n" ----
Edit id(0) label('hello world') duration(0)
---- Sent utf8 encoded message: "edit --name \"@²³ß\\\"ä\" --duration 1337\n" ----
Edit id(0) label('@²³ß"ä') duration(1337)

✔️

@dhebbeker
dhebbeker marked this pull request as ready for review November 30, 2023 20:55
@dhebbekerdhebbeker added the enhancement New feature or request label Nov 30, 2023
FiveTeethless
FiveTeethless previously approved these changes Dec 12, 2023

@FiveTeethlessFiveTeethless left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks ok for me.
I didn't go through all the changes, but it looks promising ;)

Comment threadtest/test_serial_port/test_serial_port.cpp
@dhebbeker
dhebbekerforce-pushed the feature/60-serial-interface-add-command-parser-for-protocol branch from 3906fe7 to 0626893CompareDecember 12, 2023 21:48

@dhebbekerdhebbeker left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I did rebase the branch in order to resolve merge conflicts. That means that an updated review is required.

Comment threadtest/test_serial_port/test_serial_port.cpp
@dhebbeker
dhebbeker merged commit 1647566 into developDec 17, 2023
@dhebbeker
dhebbeker deleted the feature/60-serial-interface-add-command-parser-for-protocol branch December 17, 2023 07:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

serial interface: add command parser for protocol

2 participants

@dhebbeker@FiveTeethless