-
Notifications
You must be signed in to change notification settings - Fork 164
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
Improving documentation #371
Conversation
- Adding recommended naming convention for signals, attributes and branch - Aligning whether attribute actually is a signal or not. Previously it was sometimes considered as signal, sometimes not. Now not! - Clarifying expected behavior when setting and reading actuators - Aligning documentation on how often attribute values can change Previously we said in some places that they never can change In some places that they can change, but normally not more than once per ignition cycle. Now we say that they can change. - Removing the "stream" node type. Also removing stream as this keyword no longer is supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it. It gives clearer guidance and is still in line with current practices
## Naming Conventions | ||
|
||
The recommended naming convention for node elements is to use camel case notation starting with a capital letter. It is recommended to use only | ||
`A-Z`, `a-z` and `0-9` in node names. For boolean signals it is recommended to start the name with `Is`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, the "Is*" naming convention for boolean signals works fine for sensors, but is maybe not that intuitive for actuators. Do we want to use the "Is*" naming convention only for sensors, or also for actuators. Any thoughts @gunnarx ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use it for both. In programming languages it would be a boolean field, and some code generator/syntactiv sugar would generate get/set as well as isXXX methods. But in keeping VSS simple (just a name and a datatype) "Is" is still a nice hint/convention that an element is a boolean. For that, I could live with "writing to" IsXXX
The other clean alternative would be getting rid of "Is" altogether e.h. ParkingBrake.IsEngaged -> ParkingBrake.Engaged , what I would not do is "mix" and do it differently for sensors and actuators, because actuators are sensors to, in the parking break example probably most apps would neither want nor have the permission to set it anyway. So it would make it a little non-obvious for developers whether there is "Is" or not.
SW Frameworks on top might still opt to sweeten it with nicer names.
so tldr; valid options I can see:
- "Is" for all -> would be my vote
- no "Is" at all
meeting 2021-12-14 approved |
Previously it was sometimes considered as signal, sometimes not. Now not!
Previously we said in some places that they never can change
In some places that they can change, but normally not more than once per
ignition cycle. Now we say that they can change.
Also removing stream as this keyword no longer is supported.