Skip to content

Udate architecture diagram.#93

Merged
BorjaOuterelo merged 4 commits into
masterfrom
feature/architecturediagram
Jan 23, 2020
Merged

Udate architecture diagram.#93
BorjaOuterelo merged 4 commits into
masterfrom
feature/architecturediagram

Conversation

@BorjaOuterelo
Copy link
Copy Markdown
Contributor

Update architecture diagram to follow same theme and use concepts from decision paper.

@BorjaOuterelo BorjaOuterelo force-pushed the feature/architecturediagram branch from 28b5750 to ae07d75 Compare January 21, 2020 14:51
julionce
julionce previously approved these changes Jan 22, 2020
Copy link
Copy Markdown
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

Hi @BorjaOuterelo, sorry for reviewing this PR so late. I like the details on rclc that have been added. Very good! Yet, my first visual impression was that the diagram became very complicated. I propose to remove the 3D effect completely or to reduce the depth of the boxes to a minimum. Also, in my diagram, I put the rcl extensions on the same layer as rcl to express more clearly that we basically use rcl as C API and do not create a completely new C API.

@BorjaOuterelo
Copy link
Copy Markdown
Contributor Author

BorjaOuterelo commented Jan 23, 2020

@ralph-lange thanks for the review. Already addressed your comment on clarity by:

  • Changing the 3D effect. It may be more clear now.
  • Adding some more colours and a legend trying to clarify package/source code origin.

The C API I have made the box of rcl to reach from one end to the other on the Client library. I want to state also that most of the rcl extensions use that same rcl.
Any feedback is welcome.

@ralph-lange
Copy link
Copy Markdown
Contributor

Hi @BorjaOuterelo, the diagram looks much better now. The coloring is a very good idea! I propose to place micro-ROS rclc inside the orange C API shape. Otherwise a reader might think that we want to provide three APIs (rcl, rclc, rclcpp).

@BorjaOuterelo
Copy link
Copy Markdown
Contributor Author

@ralph-lange I added your API susggestion. It seems more clear now. Right?

@ralph-lange
Copy link
Copy Markdown
Contributor

Should/may we already add FreeRTOS in the diagram?

@BorjaOuterelo
Copy link
Copy Markdown
Contributor Author

BorjaOuterelo commented Jan 23, 2020

I added it. It messes a bit the RTOS part, but it states that we have support for both, NuttX and FreeRTOS.

@BorjaOuterelo BorjaOuterelo merged commit b7d498b into master Jan 23, 2020
@BorjaOuterelo BorjaOuterelo deleted the feature/architecturediagram branch January 23, 2020 12:30
amx-piap pushed a commit that referenced this pull request May 5, 2020
* Udate architecture diagram.

* Change architecture shapes and colors.

* Improve client library API diagram.

* Add FreeRTOS.
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