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

Add Linter #103

Merged
merged 6 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
; see http://editorconfig.org/ for docs on this file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inspired by ML-Agents repo's config. We are using dotnet format to format C# code.

; See https://github.com/dotnet/format for dotnet format

root = true

[*]
ignore_if_in_header = This code was generated by a tool|<auto-generated>
indent_style = space
indent_size = 4
; uncomment to help with sharing files across os's (i.e. network share or through local vm)
#end_of_line = lf
; avoid a bom, which causes endless problems with naive text tooling
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
; keeping auto-format enabled helps avoid merge hell for projects without CI-based format validation
#disable_auto_format = true

[*.cs]
; uncomment to enable full formatting of c# files
formatters = generic, uncrustify

[*.asmdef]
scrape_api = true

[**/Tests/**.asmdef]
scrape_api = false

[*.Tests.asmdef]
scrape_api = false

[*.md]
indent_size = 2
; trailing whitespace is unfortunately significant in markdown
trim_trailing_whitespace = false
; uncomment to enable basic formatting of markdown files
#formatters = generic

[{Makefile,makefile}]
; tab characters are part of the Makefile format
indent_style = tab

[*.asmdef]
indent_size = 4

[*.json]
indent_size = 2

[*.{vcproj,bat,cmd,xaml,tt,t4,ttinclude}]
end_of_line = crlf

; this VS-specific stuff is based on experiments to see how VS will modify a file after it has been manually edited.
; the settings are meant to closely match what VS does to minimize unnecessary diffs.
[*.{vcxproj,vcxproj.filters}]
indent_style = space
indent_size = 2
end_of_line = crlf
charset = utf-8-bom
trim_trailing_whitespace = true
insert_final_newline = false
; must be broken out because of 51-char bug (https://github.com/editorconfig/editorconfig-visualstudio/issues/21)
[*.{csproj,pyproj,props,targets}]
indent_style = space
indent_size = 2
end_of_line = crlf
charset = utf-8-bom
trim_trailing_whitespace = true
insert_final_newline = false
[*.{sln,sln.template}]
indent_style = tab
indent_size = 4
end_of_line = crlf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = false
sdiao marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Provide any relevant links here.

## Testing and Verification

Please describe the tests that you ran to verify your changes. Please also provide instructions, ROS packages, and Unity project files as appropriate so we can reproduce the test environment.
Please describe the tests that you ran to verify your changes. Please also provide instructions, ROS packages, and Unity project files as appropriate so we can reproduce the test environment.

### Test Configuration:
- Unity Version: [e.g. Unity 2020.2.0f1]
Expand Down
17 changes: 17 additions & 0 deletions .github/workflows/pre-commit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: pre-commit

on:
pull_request:

jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: 3.7.x
- uses: actions/setup-dotnet@v1
with:
dotnet-version: '3.1.x'
- uses: pre-commit/[email protected]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inspired by ML-Agents team's config. We are adding a Github Check here, so that once a pull request is opened, Github will run the pre-commit check as well.

Success check example
Failed check example

31 changes: 31 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
repos:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Key pre-commit config setup.

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.0.1
hooks:
- id: mixed-line-ending
exclude: >
(?x)^(
.*cs.meta|
.*.css|
.*.meta|
.*.mat|
.*.preset|
.*.lighting
)$
args: [--fix=lf]

- id: trailing-whitespace
name: trailing-whitespace-markdown
types: [markdown]
- id: check-merge-conflict
args: [--assume-in-merge]
- id: check-yaml
# Won't handle the templating in yamato
exclude: \.yamato/.*


- repo: https://github.com/dotnet/format
rev: "7e343070a0355c86f72bdee226b5e19ffcbac931"
hooks:
- id: dotnet-format
args: [--folder, --include]
14 changes: 7 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Contribution Guidelines

Thank you for your interest in contributing to Unity Robotics! To facilitate your
contributions, we've outlined a brief set of guidelines to ensure that your extensions
Thank you for your interest in contributing to Unity Robotics! To facilitate your
contributions, we've outlined a brief set of guidelines to ensure that your extensions
can be easily integrated.

## Communication
Expand Down Expand Up @@ -40,10 +40,10 @@ We run continuous integration on all PRs; all tests must be passing before the P

All Python code should follow the [PEP 8 style guidelines](https://pep8.org/).

All C# code should follow the [Microsoft C# Coding Conventions](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions).
Additionally, the [Unity Coding package](https://docs.unity3d.com/Packages/[email protected]/manual/index.html)
can be used to format, encode, and lint your code according to the standard Unity
development conventions. Be aware that these Unity conventions will supersede the
All C# code should follow the [Microsoft C# Coding Conventions](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions).
Additionally, the [Unity Coding package](https://docs.unity3d.com/Packages/[email protected]/manual/index.html)
can be used to format, encode, and lint your code according to the standard Unity
development conventions. Be aware that these Unity conventions will supersede the
Microsoft C# Coding Conventions where applicable.

Please note that even if the code you are changing does not adhere to these guidelines,
Expand All @@ -60,5 +60,5 @@ email us at [[email protected]](mailto:[email protected]).

## Contribution review

Once you have a change ready following the above ground rules, simply make a
Once you have a change ready following the above ground rules, simply make a
pull request in GitHub.
26 changes: 13 additions & 13 deletions MessageGeneration.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# Message Generation
To work with a ROS message in Unity, you first need to generate the corresponding C# message class. Select the menu option "RosMessageGeneration->Browse..." to open the message browser.
![](images~/MessageBrowser.png)
Select your ROS message folder at the top, then you can navigate through the folder structure to find the .msg files. Click the "Build msg" button to build the messages you want.
# Message importers
For a more automated workflow, you may find it useful to simply drag an entire ROS module folder into your Unity project. Unity will automatically find any .msg and .srv files in the folder structure, and convert them into C# message classes for you. And the classes will be updated if the .msg or .srv files change on disk.
NB: The message generation system looks for a ROS package.xml to determine what code to generate, so if you're working this way it's recommended to import an entire ROS module, rather than individual .msg files, into your Unity project.
# Message Generation

To work with a ROS message in Unity, you first need to generate the corresponding C# message class. Select the menu option "RosMessageGeneration->Browse..." to open the message browser.

![](images~/MessageBrowser.png)

Select your ROS message folder at the top, then you can navigate through the folder structure to find the .msg files. Click the "Build msg" button to build the messages you want.

# Message importers

For a more automated workflow, you may find it useful to simply drag an entire ROS module folder into your Unity project. Unity will automatically find any .msg and .srv files in the folder structure, and convert them into C# message classes for you. And the classes will be updated if the .msg or .srv files change on disk.

NB: The message generation system looks for a ROS package.xml to determine what code to generate, so if you're working this way it's recommended to import an entire ROS module, rather than individual .msg files, into your Unity project.
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
[![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0)

## Installation
1. Using Unity 2020.2 or later, open the package manager from `Window` -> `Package Manager` and select "Add package from git URL..."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

markdown files format are fixed automatically. Instruction to set up: https://pre-commit.com/
To try it out locally, run pre-commit run --files *.md
Screen Shot 2021-05-20 at 5 15 37 PM

1. Using Unity 2020.2 or later, open the package manager from `Window` -> `Package Manager` and select "Add package from git URL..."
![image](https://user-images.githubusercontent.com/29758400/110989310-8ea36180-8326-11eb-8318-f67ee200a23d.png)
2. Enter the following URL. If you don't want to use the latest version, substitute your desired version tag where we've put `v0.3.0` in this example:
`https://github.com/Unity-Technologies/ROS-TCP-Connector.git?path=/com.unity.robotics.ros-tcp-connector#v0.3.0`
3. Click `Add`.


## Tutorials
## Tutorials
Scripts used to send [ROS](https://www.ros.org/) messages to an [TCP endpoint](https://github.com/Unity-Technologies/ROS_TCP_Endpoint) running as a ROS node.

This Unity package provides three main features:
Expand All @@ -27,16 +27,16 @@ Special thanks to the Siemens [ROS# Project Team]( https://github.com/siemens/ro

## Community and Feedback

The Unity Robotics projects are open-source and we encourage and welcome contributions.
If you wish to contribute, be sure to review our [contribution guidelines](CONTRIBUTING.md)
The Unity Robotics projects are open-source and we encourage and welcome contributions.
If you wish to contribute, be sure to review our [contribution guidelines](CONTRIBUTING.md)
and [code of conduct](CODE_OF_CONDUCT.md).

## Support
For questions or discussions about Unity Robotics package installations or how to best set up and integrate your robotics projects, please create a new thread on the [Unity Robotics forum](https://forum.unity.com/forums/robotics.623/) and make sure to include as much detail as possible.

For feature requests, bugs, or other issues, please file a [GitHub issue](https://github.com/Unity-Technologies/ROS-TCP-Connector/issues) using the provided templates and the Robotics team will investigate as soon as possible.

For any other questions or feedback, connect directly with the
For any other questions or feedback, connect directly with the
Robotics team at [[email protected]](mailto:[email protected]).

## License
Expand Down
112 changes: 56 additions & 56 deletions ROSGeometry.md
Original file line number Diff line number Diff line change
@@ -1,56 +1,56 @@
# ROSGeometry component
In Unity, the X axis points right, Y up, and Z forward. ROS, on the other hand, supports various coordinate frames: in the most commonly-used one, X points forward, Y left, and Z up. In ROS terminology, this frame is called "FLU" (forward, left, up), whereas the Unity coordinate frame would be "RUF" (right, up, forward).
The ROSGeometry namespace contains code to make it easier to work with these various coordinate frames - letting you be explicit about what coordinates a given value is in at compile time, and managing the conversions for you. It does this with two generic structs, `Vector3<C>` and `Quaternion<C>`. The type parameter C indicates the coordinate frame you're working in - either FLU, or RUF, or perhaps a more exotic frame such as NED (north, east, down) or ENU (east, north, up), commonly used in aviation.
# Converting between frames:
For example, if you need to convert an object's position into the FLU coordinate frame, you might write:
Vector3<FLU> rosPos = obj.transform.position.To<FLU>();
An explicit cast, or calling the constructor, would also produce the same result.
Vector3<FLU> rosPos2 = (Vector3<FLU>)obj.transform.position;
Vector3<FLU> rosPos3 = new Vector3<FLU>(obj.transform.position);
To convert back, just access the "toUnity" property on the vector.
Vector3 unityPos = rosPos.toUnity;
And the same functions apply for converting Quaternions.
# Ros Message conversions:
For convenience, `Vector3<C>` has an implicit conversion into all three of the main ROS position message types: Point, Point32 and Vector3. Similarly, `Quaternion<C>` has an implicit conversion to the ROS Quaternion message. Hence, writing 3d data into a message can often be as simple as writing:
Imu msg = new Imu();
msg.linear_acceleration = acceleration.To<FLU>();
msg.orientation = rigidbody.transform.rotation.To<FLU>();
msg.angular_velocity = rigidbody.angularVelocity.To<FLU>();
ros.Send("imu", msg);
Note, the calls to `To<FLU>()` above are essential. Normal Unity Vector3s or Quaternions do NOT have these conversions. You need to explicitly select a coordinate frame before converting to a ROS message.
Unity's standard Transform class also has a `To<C>()` extension method that returns a ROS Transform message. So sending a Transform message typically looks like:
ros.Send("topic", obj.transform.To<FLU>());
# Converting incoming messages
You can also convert Points, Point32s and Vector3s back into Unity coordinates. To convert a Point in coordinate space C directly into a Unity Vector3, you can write `From<C>`. For example:
void SubscriberCallback(Point p)
{
transform.position = p.From<FLU>();
}
Or, if you need to work with them in the FLU coordinate space for now, you can write:
Vector3<FLU> rosPos = p.As<FLU>();
(Note that this does NOT do any coordinate conversion. It simply assumes the point is in the FLU coordinate frame already, and transfers it into an appropriate container.)
And again, the same goes for converting a Quaternion message into a Unity Quaternion or `Quaternion<C>`.
# ROSGeometry component

In Unity, the X axis points right, Y up, and Z forward. ROS, on the other hand, supports various coordinate frames: in the most commonly-used one, X points forward, Y left, and Z up. In ROS terminology, this frame is called "FLU" (forward, left, up), whereas the Unity coordinate frame would be "RUF" (right, up, forward).

The ROSGeometry namespace contains code to make it easier to work with these various coordinate frames - letting you be explicit about what coordinates a given value is in at compile time, and managing the conversions for you. It does this with two generic structs, `Vector3<C>` and `Quaternion<C>`. The type parameter C indicates the coordinate frame you're working in - either FLU, or RUF, or perhaps a more exotic frame such as NED (north, east, down) or ENU (east, north, up), commonly used in aviation.


# Converting between frames:

For example, if you need to convert an object's position into the FLU coordinate frame, you might write:

Vector3<FLU> rosPos = obj.transform.position.To<FLU>();

An explicit cast, or calling the constructor, would also produce the same result.

Vector3<FLU> rosPos2 = (Vector3<FLU>)obj.transform.position;
Vector3<FLU> rosPos3 = new Vector3<FLU>(obj.transform.position);

To convert back, just access the "toUnity" property on the vector.

Vector3 unityPos = rosPos.toUnity;
sdiao marked this conversation as resolved.
Show resolved Hide resolved

And the same functions apply for converting Quaternions.

# Ros Message conversions:

For convenience, `Vector3<C>` has an implicit conversion into all three of the main ROS position message types: Point, Point32 and Vector3. Similarly, `Quaternion<C>` has an implicit conversion to the ROS Quaternion message. Hence, writing 3d data into a message can often be as simple as writing:

Imu msg = new Imu();
msg.linear_acceleration = acceleration.To<FLU>();
msg.orientation = rigidbody.transform.rotation.To<FLU>();
msg.angular_velocity = rigidbody.angularVelocity.To<FLU>();
ros.Send("imu", msg);
Comment on lines +29 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

use var and a more descriptive name than msg (note: msg abbreviation is ok)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would it be good to keep this PR only adding the linter and formatting? Happy to open a separate PR to update the sample code in markdown files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks we already have a task AIRO-667 to update the doc. Doc changes will be made in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok when I expand the code I can see that this "addition" is really just moving it around. Our current rule is that if the code is new, or if you are re-writing older code for some reason, it must be changed to follow the coding standards if it doesn't already. Since this is technically a change and not a move, it's ok to defer.


Note, the calls to `To<FLU>()` above are essential. Normal Unity Vector3s or Quaternions do NOT have these conversions. You need to explicitly select a coordinate frame before converting to a ROS message.

Unity's standard Transform class also has a `To<C>()` extension method that returns a ROS Transform message. So sending a Transform message typically looks like:

ros.Send("topic", obj.transform.To<FLU>());

# Converting incoming messages

You can also convert Points, Point32s and Vector3s back into Unity coordinates. To convert a Point in coordinate space C directly into a Unity Vector3, you can write `From<C>`. For example:

void SubscriberCallback(Point p)
sdiao marked this conversation as resolved.
Show resolved Hide resolved
{
transform.position = p.From<FLU>();
}

Or, if you need to work with them in the FLU coordinate space for now, you can write:

Vector3<FLU> rosPos = p.As<FLU>();
sdiao marked this conversation as resolved.
Show resolved Hide resolved

(Note that this does NOT do any coordinate conversion. It simply assumes the point is in the FLU coordinate frame already, and transfers it into an appropriate container.)

And again, the same goes for converting a Quaternion message into a Unity Quaternion or `Quaternion<C>`.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Unity.Robotics.ROSTCPConnector.MessageGeneration;
Expand Down Expand Up @@ -269,7 +269,7 @@ CachedEntry CacheFolder(string path)
if (numActions > 0)
buildStrings.Add(numActions + " action" + (numActions > 1 ? "s" : ""));

if(buildStrings.Count > 0)
if (buildStrings.Count > 0)
buildLabel = "Build " + string.Join(", ", buildStrings.Select(x => x.ToString()));

return new CachedEntry()
Expand All @@ -294,7 +294,7 @@ CachedEntryStatus GetFileStatus(string path)
case ".action":
string[] builtActionPaths = ActionAutoGen.GetActionClassPaths(path, MessageGenBrowserSettings.Get().outputPath);
return builtActionPaths.All(file => File.Exists(file)) ? CachedEntryStatus.BuiltActionFile : CachedEntryStatus.UnbuiltActionFile;
}
}

return CachedEntryStatus.Ignored;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static string ToRelativePath(string fullPath)
if (!fullPath.StartsWith(dataPath))
return "";

return fullPath.Substring(dataPath.Length+1);
return fullPath.Substring(dataPath.Length + 1);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ public class ScriptedMsgImporter : ScriptedImporter
{
public override void OnImportAsset(AssetImportContext ctx)
{
string inputPath = Path.Combine(Directory.GetCurrentDirectory(), ctx.assetPath);
string inputPath = Path.Combine(Directory.GetCurrentDirectory(), ctx.assetPath);
string outputPath = MessageGenBrowserSettings.Get().outputPath;
MessageAutoGen.GenerateSingleMessage(inputPath, outputPath);

string builtPath = MessageAutoGen.GetMessageClassPath(inputPath, outputPath);
string builtAssetPath = Path.Combine("Assets", MessageGenBrowserSettings.ToRelativePath(builtPath));
AssetDatabase.ImportAsset(builtAssetPath);
Object messageClass = AssetDatabase.LoadAssetAtPath(builtAssetPath, typeof(MonoScript));
if(messageClass != null)
if (messageClass != null)
ctx.AddObjectToAsset("messageClass", messageClass);
}
}
Expand Down
Loading