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

Use json serialization for socket API #80

Merged
merged 1 commit into from
Sep 28, 2020

Conversation

marcmo
Copy link
Member

@marcmo marcmo commented Aug 14, 2020

No description provided.

@marcmo marcmo requested a review from bzld August 14, 2020 07:02
@marcmo marcmo changed the title Pr runtime api Use protobuf serialization for socket API Aug 14, 2020
@marcmo marcmo force-pushed the pr_runtime_api branch 2 times, most recently from 7c43c4f to 8c78392 Compare August 14, 2020 17:44
@marcmo marcmo marked this pull request as ready for review August 14, 2020 17:52
Copy link
Collaborator

@bzld bzld left a comment

Choose a reason for hiding this comment

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

The general change is fine, but we need to fix the missing length framing to prevent serious errors, and probably should refactor the .proto file a bit to make it more structured.

(Didn't fully review each command code for client and command handler yet)

Cargo.toml Show resolved Hide resolved
north_api/src/proto/api.proto Outdated Show resolved Hide resolved
north_api/src/proto/api.proto Outdated Show resolved Hide resolved
north_api/src/proto/api.proto Outdated Show resolved Hide resolved
north_api/src/proto/api.proto Outdated Show resolved Hide resolved
north/src/command_processor.rs Outdated Show resolved Hide resolved
north/src/command_processor.rs Outdated Show resolved Hide resolved
north/src/command_processor.rs Outdated Show resolved Hide resolved
north_client/src/main.rs Outdated Show resolved Hide resolved
north/src/command_processor.rs Outdated Show resolved Hide resolved
@marcmo marcmo requested a review from bzld August 30, 2020 07:35
@marcmo marcmo marked this pull request as draft September 11, 2020 07:14
@marcmo marcmo changed the title Use protobuf serialization for socket API Use json serialization for socket API Sep 15, 2020
@marcmo marcmo requested review from flxo and removed request for bzld September 15, 2020 14:48
@marcmo marcmo marked this pull request as ready for review September 15, 2020 14:49
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
north/src/main.rs Outdated Show resolved Hide resolved
rakefile.rb Outdated Show resolved Hide resolved
north/src/console.rs Outdated Show resolved Hide resolved
north_api/src/api.rs Outdated Show resolved Hide resolved
north_api/src/api.rs Outdated Show resolved Hide resolved
north_api/src/api.rs Outdated Show resolved Hide resolved
north_api/src/api.rs Outdated Show resolved Hide resolved
north_api/src/api.rs Outdated Show resolved Hide resolved
* Add new data model
* Usage of json api in console
* new client based on json api
* Serialization for Socket API with serde-json
* Remove old dcon client
* Move help to client
* Add basic info to README
* Add length to frames -- we pack the complete
  message into a frame that contains the length
  in 4 bytes before the actual message
* Add tests for frame header
* Add rake-task to do a debug build
* Use uniqu hid for start/stop/uninstall
  (no regex is constructed anymore)
  makes it unambigous which component is meant
* add request/response id pairs
* add checks for matching request-response id pairs
* move api to north_common
* move API functionality out of command_processor
  into state so that it might be reused
* update API documentation
* Fix clippy warnings
* add clippy nightly checks to rake task
@marcmo marcmo merged commit 25997d1 into esrlabs:master Sep 28, 2020
@marcmo marcmo deleted the pr_runtime_api branch September 29, 2020 06:41
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.

3 participants