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

Review chapters 1, 2, 3, 4, 6 #16

Closed
pavel-kirienko opened this issue Aug 21, 2018 · 8 comments
Closed

Review chapters 1, 2, 3, 4, 6 #16

pavel-kirienko opened this issue Aug 21, 2018 · 8 comments
Assignees
Labels
help wanted Extra attention is needed

Comments

@pavel-kirienko
Copy link
Member

The spec draft is ready except for the chapter 5 (Application layer). The work on the application layer is blocked by the lack of the new standard data type set, so that seems to be the next thing to do.

The document has been restructured rather considerably compared to the v0 spec published on the website. The new structure provides dedicated chapters for the transport layer, the application layer, and the physical layer. Also, per Kjetil's suggestion, CAN-specific information has been extracted into two well-isolated sections (one for the transport layer and the other for the physical layer), allowing for future support of other transports.

The CAN FD physical layer compatibility recommendations are missing. They are going to remain this way until we've ran some tests with CAN FD in the lab here; nevertheless, relevant discussions and suggestions are highly welcome.

The current draft could probably use some refinement in the following areas (help needed):

  • The specification of the DSDL grammar (sections 3.2, 3.3, 3.4) is rather haphazard. I believe it could greatly benefit from a complete rewrite from scratch. The main reason it ended up in this state is that the early draft was updated and amended in various ways multiple times until the current edition was settled upon. Now, having the design solidified, is a good opportunity to rework it into a proper specification. It is not a high priority task but is something I could use help with.
  • Section 4.3 (transport layer - transfer reception) could be incomplete or hard to understand. Transfer reconstruction is probably the hardest part of the protocol to implement right, so it is important to ensure that this part is comprehensible and is unambiguous.

Here is the latest draft (built from the current master): UAVCAN_Specification.pdf

I think as we are slowly approaching completion of the document I should stop cowboying and begin using a more disciplined approach to editing with feature branches, especially if you're going to edit the document alongside.

@pavel-kirienko pavel-kirienko added the help wanted Extra attention is needed label Aug 21, 2018
@kjetilkjeka
Copy link
Contributor

This is a pretty big issue and I'm probably going to "proofread" and bring up concerns a bit spordically. Since this is a big issue we should probably let this branch out into different issue. On top of my head right now i have:

@thirtytwobits
Copy link
Member

I have started an in-depth review of this version. I'll post a PDF with my notes when I complete it. This could take awhile.

@pavel-kirienko
Copy link
Member Author

I'm beginning to think that the PDF-based approach is not quite the best option after all. Perhaps you could open new issues for every proposed change instead? For simple no-brainer changes, submitting PR's directly is the best approach. Especially with the new Kjetil's build bot.

@thirtytwobits
Copy link
Member

FYI: the "DRAFT" watermark makes it really difficult to comment on some text because it's selectable. Is there anyway to make it non-selectable? If not can we just make this a header and/or footer?

@thirtytwobits
Copy link
Member

Okay. I'll start converting the notes I've been taking in the PDF to either issues or PRs.

@pavel-kirienko
Copy link
Member Author

I apologize for being out of touch the past several days - had other stuff happening at work and then suddenly got flu. I am back now, slowly working through the unanswered mail.

@thirtytwobits
Copy link
Member

Sorry to hear. Hope you are feeling better!

thirtytwobits added a commit that referenced this issue Sep 11, 2018
Mostly English fixes including proper tex em dashes and quotes.
Some substantiative fixes as well.
pavel-kirienko added a commit that referenced this issue Sep 19, 2018
@pavel-kirienko
Copy link
Member Author

I believe this is no longer relevant, closing. Please re-open if I am mistaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants