Repository: ros-tooling/cross_compile

Issue:
Add supported cross compile targets to README
### Description
Supported target platforms should be easily located in the README doc so users of Cross Compile can quickly know what is supported.

### Test Plan
* N/A

### Documentation
* Update `README.md` to display supported cross compile targets

### Release Plan
* check in PR

### Acceptance Criteria
* [ ] `README.md` is updated
* [ ] Release plan has been completed

Once all above items are checked, this story can be moved to done.

### Resources

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=h1) Report
> Merging [#116](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=desc) into [master](https://codecov.io/gh/ros-tooling/cross_compile/commit/5461a52cb970dd0ee4318e1de733cb7351dc53f8?src=pr&el=desc) will **decrease** coverage by `0.85%`.
> The diff coverage is `75%`.

[![Impacted file tree graph](https://codecov.io/gh/ros-tooling/cross_compile/pull/116/graphs/tree.svg?width=650&token=cAWnYaNaLz&height=150&src=pr)](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage   86.13%   85.27%   -0.86%     
==========================================
  Files           6        6              
  Lines         339      360      +21     
==========================================
+ Hits          292      307      +15     
- Misses         47       53       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #unittests | `85.27% <75%> (-0.86%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [test/test\_sysroot\_compiler.py](https://codecov.io/gh/ros-tooling/cross_compile/pull/116/diff?src=pr&el=tree#diff-dGVzdC90ZXN0X3N5c3Jvb3RfY29tcGlsZXIucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [cross\_compile/ros2\_cross\_compile.py](https://codecov.io/gh/ros-tooling/cross_compile/pull/116/diff?src=pr&el=tree#diff-Y3Jvc3NfY29tcGlsZS9yb3MyX2Nyb3NzX2NvbXBpbGUucHk=) | `69.23% <50%> (ø)` | :arrow_up: |
| [cross\_compile/sysroot\_compiler.py](https://codecov.io/gh/ros-tooling/cross_compile/pull/116/diff?src=pr&el=tree#diff-Y3Jvc3NfY29tcGlsZS9zeXNyb290X2NvbXBpbGVyLnB5) | `77.4% <73.52%> (-0.95%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=footer). Last update [5461a52...0c93d62](https://codecov.io/gh/ros-tooling/cross_compile/pull/116?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 95947f3..84c4b22 100644
--- a/README.md
+++ b/README.md
@@ -3,11 +3,21 @@
 ![License](https://img.shields.io/github/license/ros-tooling/cross_compile)
 [![Documentation Status](https://readthedocs.org/projects/cross_compile/badge/?version=latest)](https://cross_compile.readthedocs.io/en/latest/?badge=latest)
 
-A tool to automate ROS2 packages compilation to non-native architectures.
+A tool to automate compiling ROS and ROS2 workspaces to non-native architectures.
 
 :construction: `cross_compile` relies on running emulated builds
 using QEmu, #69 tracks progress toward enabling cross-compilation.
 
+## Supported targets
+
+This tool supports compiling a workspace for all combinations of the following:
+
+* Architecture: `armhf`, `aarch64`
+* ROS Distro
+  * ROS: `kinetic`, `melodic`
+  * ROS 2: `dashing`, `eloquent`
+* OS: `Ubuntu`, `Debian`
+
 ## Installation
 
 ### Prerequisites
diff --git a/cross_compile/ros2_cross_compile.py b/cross_compile/ros2_cross_compile.py
index 7f032bb..2a8e303 100644
--- a/cross_compile/ros2_cross_compile.py
+++ b/cross_compile/ros2_cross_compile.py
@@ -38,14 +38,8 @@ def parse_args(args: List[str]) -> argparse.Namespace:
         '-a', '--arch',
         required=True,
         type=str,
-        choices=['armhf', 'aarch64'],
+        choices=Platform.SUPPORTED_ARCHITECTURES.keys(),
         help='Target architecture')
-    parser.add_argument(
-        '-o', '--os',
-        required=True,
-        type=str,
-        choices=['ubuntu', 'debian'],
-        help='Target OS')
     parser.add_argument(
         '-d', '--rosdistro',
         required=False,
@@ -53,6 +47,12 @@ def parse_args(args: List[str]) -> argparse.Namespace:
         default='dashing',
         choices=Platform.SUPPORTED_ROS_DISTROS + Platform.SUPPORTED_ROS2_DISTROS,
         help='Target ROS distribution')
+    parser.add_argument(
+        '-o', '--os',
+        required=True,
+        type=str,
+        # NOTE: not specifying choices here, as different distros may support different lists
+        help='Target OS')
     parser.add_argument(
         '-r', '--rmw',
         required=False,
@@ -64,7 +64,8 @@ def parse_args(args: List[str]) -> argparse.Namespace:
         '--sysroot-base-image',
         required=False,
         type=str,
-        help='Base Docker image to use for building the sysroot. Ex. arm64v8/ubuntu:bionic')
+        help='Override the default base Docker image to use for building the sysroot. '
+             'Ex. "arm64v8/ubuntu:bionic"')
     parser.add_argument(
         '--docker-network-mode',
         required=False,
@@ -85,7 +86,6 @@ def parse_args(args: List[str]) -> argparse.Namespace:
         help="The subdirectory of 'sysroot' that contains your 'src' to be built."
              'The output of the cross compilation will be placed in this directory. '
              "Defaults to 'ros_ws'.")
-
     parser.add_argument(
         '--sysroot-path',
         required=False,
@@ -120,11 +120,12 @@ def main():
     """Start the cross-compilation workflow."""
     # Configuration
     args = parse_args(sys.argv[1:])
-    platform = Platform(
-        args.arch, args.os, args.rosdistro, args.rmw)
+    platform = Platform(args.arch, args.os, args.rosdistro, args.rmw)
     docker_args = DockerConfig(
-        args.arch, args.os, args.rosdistro, args.sysroot_base_image,
-        args.docker_network_mode, args.sysroot_nocache)
+        platform,
+        args.sysroot_base_image,
+        args.docker_network_mode,
+        args.sysroot_nocache)
 
     # Main pipeline
     sysroot_create = SysrootCompiler(cc_root_dir=args.sysroot_path,
diff --git a/cross_compile/sysroot_compiler.py b/cross_compile/sysroot_compiler.py
index 5f6b660..cba75b2 100755
--- a/cross_compile/sysroot_compiler.py
+++ b/cross_compile/sysroot_compiler.py
@@ -22,6 +22,7 @@ import shutil
 from string import Template
 import tarfile
 import tempfile
+from typing import NamedTuple
 from typing import Optional
 
 import docker
@@ -59,7 +60,14 @@ logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger(__name__)
 
 
-def replace_tree(src: Path, dest: Path) -> None:
+class ArchMapping(NamedTuple):
+    """Pair the toolchain and base Docker image for a target architecture as named members."""
+
+    toolchain: str
+    docker_base: str
+
+
+def _replace_tree(src: Path, dest: Path) -> None:
     """Delete dest and copy the directory src to that location."""
     shutil.rmtree(str(dest), ignore_errors=True)  # it may or may not exist already
     shutil.copytree(str(src), str(dest))
@@ -76,29 +84,91 @@ class Platform:
     4. RMW implementation used
     """
 
+    # NOTE: when changing any following values, update README.md Supported Targets section
+    SUPPORTED_ARCHITECTURES = {
+        'armhf': ArchMapping(
+            toolchain='arm-linux-gnueabihf',
+            docker_base='arm32v7',
+        ),
+        'aarch64': ArchMapping(
+            toolchain='aarch64-linux-gnu',
+            docker_base='arm64v8',
+        )
+    }
+
     SUPPORTED_ROS2_DISTROS = ['dashing', 'eloquent']
     SUPPORTED_ROS_DISTROS = ['kinetic', 'melodic']
 
-    def __init__(self, arch: str, os: str, rosdistro: str, rmw: str):
+    ROSDISTRO_OS_MAPPING = {
+        'kinetic': {
+            'ubuntu': 'xenial',
+            'debian': 'jessie',
+        },
+        'melodic': {
+            'ubuntu': 'bionic',
+            'debian': 'stretch',
+        },
+        'dashing': {
+            'ubuntu': 'bionic',
+            'debian': 'stretch',
+        },
+        'eloquent': {
+            'ubuntu': 'bionic',
+            'debian': 'buster',
+        },
+    }
+    # NOTE: when changing any preceding values, update README.md Supported Targets section
+
+    def __init__(self, arch: str, os_name: str, rosdistro: str, rmw: str):
         """Initialize platform parameters."""
-        self.arch = arch
-        self.os = os
-        self.rosdistro = rosdistro
-        self.rmw = rmw
+        self._arch = arch
+        self._rosdistro = rosdistro
+        self._os_name = os_name
+        self._rmw = rmw
 
-        if self.arch == 'armhf':
-            self.cc_toolchain = 'arm-linux-gnueabihf'
-        elif self.arch == 'aarch64':
-            self.cc_toolchain = 'aarch64-linux-gnu'
+        try:
+            self._cc_toolchain = self.SUPPORTED_ARCHITECTURES[arch].toolchain
+        except KeyError:
+            raise ValueError('Unknown target architecture "{}" specified'.format(arch))
 
         if self.rosdistro in self.SUPPORTED_ROS2_DISTROS:
-            self.ros_version = 'ros2'
+            self._ros_version = 'ros2'
         elif self.rosdistro in self.SUPPORTED_ROS_DISTROS:
-            self.ros_version = 'ros'
+            self._ros_version = 'ros'
+        else:
+            raise ValueError('Unknown ROS distribution "{}" specified'.format(rosdistro))
+
+        if self.os_name not in self.ROSDISTRO_OS_MAPPING[self.rosdistro]:
+            raise ValueError(
+                'OS "{}" not supported for ROS distro "{}"'.format(os_name, rosdistro))
+
+    @property
+    def arch(self):
+        return self._arch
+
+    @property
+    def rosdistro(self):
+        return self._rosdistro
+
+    @property
+    def os_name(self):
+        return self._os_name
+
+    @property
+    def rmw(self):
+        return self._rmw
+
+    @property
+    def cc_toolchain(self):
+        return self._cc_toolchain
+
+    @property
+    def ros_version(self):
+        return self._ros_version
 
     def __str__(self):
         """Return string representation of platform parameters."""
-        return '-'.join((self.arch, self.os, self.rosdistro))
+        return '-'.join((self.arch, self.os_name, self.rosdistro))
 
     def get_workspace_image_tag(self) -> str:
         """Generate docker image name and tag."""
@@ -116,38 +186,18 @@ class DockerConfig:
     """
 
     def __init__(
-        self, arch: str, os: str, rosdistro: str, sysroot_base_image: str,
+        self, platform: Platform, override_base_image: Optional[str],
         docker_network_mode: str, sysroot_nocache: bool
     ):
-        base_image = {
-            'armhf': 'arm32v7',
-            'aarch64': 'arm64v8',
-        }
-        image_tag = {
-            'kinetic': {
-                'ubuntu': 'xenial',
-                'debian': 'jessie',
-            },
-            'melodic': {
-                'ubuntu': 'bionic',
-                'debian': 'stretch',
-            },
-            'dashing': {
-                'ubuntu': 'bionic',
-                'debian': 'stretch',
-            },
-            'eloquent': {
-                'ubuntu': 'bionic',
-                'debian': 'buster',
-            }
-        }
-
         """Initialize docker configuration."""
-        if sysroot_base_image is None:
-            self.base_image = \
-                self.base_image = '{}/{}:{}'.format(base_image[arch], os, image_tag[rosdistro][os])
+        if override_base_image:
+            self.base_image = override_base_image
         else:
-            self.base_image = sysroot_base_image
+            # Platform constructor has already performed validation on these values
+            docker_base = Platform.SUPPORTED_ARCHITECTURES[platform.arch].docker_base
+            distro_os = Platform.ROSDISTRO_OS_MAPPING[platform.rosdistro]
+            image_tag = distro_os[platform.os_name]
+            self.base_image = '{}/{}:{}'.format(docker_base, platform.os_name, image_tag)
 
         self.network_mode = docker_network_mode
         self.nocache = sysroot_nocache
@@ -266,7 +316,7 @@ class SysrootCompiler:
 
         mixins_src = package_path / 'mixins'
         mixins_dest = self._target_sysroot / 'mixins'
-        replace_tree(mixins_src, mixins_dest)
+        _replace_tree(mixins_src, mixins_dest)
         if not mixins_dest.exists():
             raise FileNotFoundError('Mixins not properly copied to build context')
         logger.debug('Copied mixins')


Test patch:
diff --git a/test/test_sysroot_compiler.py b/test/test_sysroot_compiler.py
index 2727e3f..ab1d9ea 100644
--- a/test/test_sysroot_compiler.py
+++ b/test/test_sysroot_compiler.py
@@ -32,27 +32,24 @@ import pytest
 THIS_SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
 
 
-def _default_docker_kwargs() -> dict:
-    return {
-        'arch': 'aarch64',
-        'os': 'ubuntu',
-        'rosdistro': 'dashing',
-        'sysroot_base_image': '035662560449.dkr.ecr.us-east-2.amazonaws.com/cc-tool:'
-                              'aarch64-bionic-dashing-fastrtps-prebuilt',
-        'docker_network_mode': 'host',
-        'sysroot_nocache': False,
-    }
-
-
 @pytest.fixture
 def platform_config() -> Platform:
     return Platform(
         arch='aarch64',
-        os='ubuntu',
+        os_name='ubuntu',
         rosdistro='dashing',
         rmw='fastrtps')
 
 
+def _default_docker_kwargs() -> dict:
+    return {
+        'platform': Platform('aarch64', 'ubuntu', 'dashing', 'fastrtps'),
+        'override_base_image': 'arm64v8/gcc:9.2.0',
+        'docker_network_mode': 'host',
+        'sysroot_nocache': False,
+    }
+
+
 @pytest.fixture
 def docker_config() -> DockerConfig:
     return DockerConfig(**_default_docker_kwargs())
@@ -71,6 +68,24 @@ def setup_mock_sysroot(path: Path) -> Tuple[Path, Path]:
     return sysroot_dir, ros_workspace_dir
 
 
+def test_platform_argument_validation():
+    rmw = 'fastrtps'
+    p = Platform('armhf', 'ubuntu', 'dashing', rmw)
+    assert p
+
+    with pytest.raises(ValueError):
+        # invalid arch
+        p = Platform('mips', 'ubuntu', 'dashing', rmw)
+
+    with pytest.raises(ValueError):
+        # invalid distro
+        p = Platform('armhf', 'ubuntu', 'ardent', rmw)
+
+    with pytest.raises(ValueError):
+        # invalid OS
+        p = Platform('armhf', 'rhel', 'dashing', rmw)
+
+
 def test_get_workspace_image_tag(platform_config):
     """Make sure the image tag is created correctly."""
     image_tag = platform_config.get_workspace_image_tag()
@@ -87,7 +102,7 @@ def test_docker_config_args(docker_config):
         'Network Mode: {}\n'
         'Caching: {}'
     ).format(
-        args['sysroot_base_image'], args['docker_network_mode'], args['sysroot_nocache']
+        args['override_base_image'], args['docker_network_mode'], args['sysroot_nocache']
     )
     config_string = str(docker_config)
     assert isinstance(config_string, str)
@@ -179,11 +194,12 @@ def test_sysroot_compiler_tree_additions(platform_config, docker_config, tmpdir)
 
 def verify_base_docker_images(arch, os, rosdistro, image_name):
     """Assert correct base image is generated."""
-    sysroot_base_image = None
+    override_base_image = None
     docker_network_mode = 'host'
     sysroot_nocache = 'False'
+    platform = Platform(arch, os, rosdistro, 'fastrtps')
     assert DockerConfig(
-        arch, os, rosdistro, sysroot_base_image,
+        platform, override_base_image,
         docker_network_mode, sysroot_nocache).base_image == image_name