Skip to content

Commit

Permalink
C++: Fix incorrect handling of blank lines in doc comments; fix a min…
Browse files Browse the repository at this point in the history 
…or mistake introduced in the prior PR (OpenCyphal#290)

- Do not lose blank lines in doc comments.
- Move `Request`/`Response` aliases out of the `_traits_`; this was
changed by mistake in OpenCyphal#287
- Repair the vscode run/debug configuration

The naming of the service pseudo-type
`Service_{{major}}_{{minor}}` non-obvious and I would like to suggest
renaming it such that the service pseudo-type is named after the service
itself suffixed with the version numbers. This would be in line with the
way it is implemented in Python. Instead of this:

```c++
uavcan::_register::Access::Request_1_0
uavcan::_register::Access::Response_1_0
uavcan::_register::Access::Service_1_0
```

We now have this:

```c++
uavcan::_register::Access::Request_1_0
uavcan::_register::Access::Response_1_0
uavcan::_register::Access_1_0
```

Then there already are aliases inside `Access_1_0` which means that the
full set of the available names is as follows:

```c++
uavcan::_register::Access::Request_1_0
uavcan::_register::Access::Response_1_0
uavcan::_register::Access_1_0
uavcan::_register::Access_1_0::Request
uavcan::_register::Access_1_0::Response
```

See related discussion in OpenCyphal#155 and OpenCyphal#190
  • Loading branch information
@pavel-kirienko
pavel-kirienko authored Mar 5, 2023
1 parent 6c0a42a commit 9dafc1c
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 203 deletions.
11 changes: 5 additions & 6 deletions .vscode/launch.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@
"version": "0.2.0",
"configurations": [
{
"name": "Python: nnvg",
"name": "Python: nnvg c++",
"type": "python",
"request": "launch",
"program": "nnvg",
"module": "nunavut",
"cwd": "${workspaceFolder}/src",
"args": ["--templates", "${workspaceFolder}/test/gentest_dsdl/templates",
"--generate-namespace-types",
"args": ["--experimental-languages",
"--verbose",
"--outdir", "${workspaceFolder}/nunavut_out",
"-l", "c",
"${workspaceFolder}/test/gentest_dsdl/dsdl/uavcan"]
"-l", "cpp",
"${workspaceFolder}/submodules/public_regulated_data_types/uavcan"]
},
{
"name": "Pytest: current test",
Expand Down
2 changes: 1 addition & 1 deletion src/nunavut/_version.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
.. autodata:: __version__
"""

__version__ = "2.0.4"
__version__ = "2.0.5"
__license__ = "MIT"
__author__ = "OpenCyphal"
__copyright__ = "Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. Copyright (c) 2022 OpenCyphal."
Expand Down
180 changes: 4 additions & 176 deletions src/nunavut/lang/cpp/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,181 +994,6 @@ def filter_declaration(language: Language, instance: pydsdl.Any) -> str:
return filter_full_reference_name(language, instance)


@template_language_filter(__name__)
def filter_definition_begin(language: Language, instance: pydsdl.CompositeType) -> str:
"""
Emit the start of a definition statement for a composite type.
.. invisible-code-block: python
from nunavut.lang.cpp import filter_definition_begin
from unittest.mock import MagicMock
import pytest
import pydsdl
my_type = MagicMock(spec=pydsdl.StructureType)
my_type.version = MagicMock()
my_type.parent_service = None
my_type.has_parent_service = False
with pytest.raises(ValueError):
jinja_filter_tester(filter_definition_begin,
'{{ my_type | definition_begin }}',
'',
'cpp',
my_type=MagicMock())
.. code-block:: python
# Given a pydsdl.CompositeType "my_type":
my_type.short_name = 'Foo'
my_type.version.major = 1
my_type.version.minor = 0
# and
template = '{{ my_type | definition_begin }}'
# then
rendered = 'struct Foo_1_0'
.. invisible-code-block: python
jinja_filter_tester(filter_definition_begin, template, rendered, 'cpp', my_type=my_type)
my_union_type = MagicMock(spec=pydsdl.UnionType)
my_union_type.version = MagicMock()
my_union_type.parent_service = None
my_union_type.has_parent_service = False
.. code-block:: python
# Also, given a pydsdl.UnionType "my_union_type":
my_union_type.short_name = 'Foo'
my_union_type.version.major = 1
my_union_type.version.minor = 0
# and
union_template = '{{ my_union_type | definition_begin }}'
# then
rendered = 'struct Foo_1_0'
.. invisible-code-block: python
jinja_filter_tester(filter_definition_begin, union_template, rendered, 'cpp', my_union_type=my_union_type)
my_service_type = MagicMock(spec=pydsdl.ServiceType)
my_service_type.version = MagicMock()
my_service_type.parent_service = None
my_service_type.has_parent_service = False
my_service_type.request_type = MagicMock(spec=pydsdl.StructureType)
my_service_type.request_type.short_name = "Request"
my_service_type.request_type.version = my_service_type.version
my_service_type.request_type.parent_service = my_service_type
my_service_type.request_type.has_parent_service = True
my_service_type.response_type = MagicMock(spec=pydsdl.StructureType)
my_service_type.response_type.short_name = "Response"
my_service_type.response_type.version = my_service_type.version
my_service_type.response_type.parent_service = my_service_type
my_service_type.response_type.has_parent_service = True
.. code-block:: python
# Finally, given a pydsdl.ServiceType "my_service_type":
my_service_type.short_name = 'Foo'
my_service_type.version.major = 1
my_service_type.version.minor = 0
# and
template = '''
{{ my_service_type | definition_begin }};
{{ my_service_type.request_type | definition_begin }};
{{ my_service_type.response_type | definition_begin }};
'''
# then
rendered = '''
namespace Foo;
struct Request_1_0;
struct Response_1_0;
'''
.. invisible-code-block: python
jinja_filter_tester(filter_definition_begin, template, rendered, 'cpp', my_service_type=my_service_type)
"""
short_name = filter_short_reference_name(language, instance)
if (
isinstance(instance, pydsdl.DelimitedType)
or isinstance(instance, pydsdl.StructureType)
or isinstance(instance, pydsdl.UnionType)
):
return "struct {}".format(short_name)
elif isinstance(instance, pydsdl.ServiceType):
return "namespace {}".format(short_name)
else:
raise ValueError("{} types cannot be redefined.".format(type(instance).__name__))


@template_language_filter(__name__)
def filter_definition_end(language: Language, instance: pydsdl.CompositeType) -> str:
"""
Emit the end of a definition statement for a composite type.
.. invisible-code-block: python
from nunavut.lang.cpp import filter_definition_end
from unittest.mock import MagicMock
import pytest
import pydsdl
with pytest.raises(ValueError):
jinja_filter_tester(filter_definition_end, '{{ my_type | definition_end }}', '', 'cpp', my_type=MagicMock())
my_type = MagicMock(spec=pydsdl.StructureType)
my_type.version = MagicMock()
my_type.short_name = 'Foo'
my_type.version.major = 1
my_type.version.minor = 0
jinja_filter_tester(filter_definition_end, '{{ my_type | definition_end }}', ';', 'cpp', my_type=my_type)
my_type = MagicMock(spec=pydsdl.UnionType)
my_type.version = MagicMock()
my_type.short_name = 'Foo'
my_type.version.major = 1
my_type.version.minor = 0
jinja_filter_tester(filter_definition_end, '{{ my_type | definition_end }}', ';', 'cpp', my_type=my_type)
my_type = MagicMock(spec=pydsdl.ServiceType)
my_type.version = MagicMock()
my_type.parent_service = None
my_type.short_name = 'Foo'
my_type.version.major = 1
my_type.version.minor = 0
jinja_filter_tester(filter_definition_end,
'{{ my_type | definition_end }}',
' // namespace Foo_1_0',
'cpp',
my_type=my_type)
"""
if (
isinstance(instance, pydsdl.DelimitedType)
or isinstance(instance, pydsdl.StructureType)
or isinstance(instance, pydsdl.UnionType)
):
return ";"
elif isinstance(instance, pydsdl.ServiceType):
return " // namespace {}".format(language.filter_short_reference_name(instance))
else:
raise ValueError("{} types cannot be redefined.".format(type(instance).__name__))


@template_language_filter(__name__)
def filter_type_from_primitive(language: Language, value: pydsdl.PrimitiveType) -> str:
"""
Expand Down Expand Up @@ -1543,7 +1368,10 @@ def _make_block_comment(text: str, prefix: str, comment: str, suffix: str, inden
tw = _make_textwrap(width=line_length, initial_indent=indented_comment, subseqent_indent=indented_comment)

for docline in doc_lines:
commented_doc_lines.extend(tw.wrap(docline))
# The docs for textwrap.TextWrapper.wrap say:
# "If the wrapped output has no content, the returned list is empty."
# This behavior cannot be altered so we need to work around it manually.
commented_doc_lines.extend(tw.wrap(docline) if docline.strip() else [indented_comment])

if len(suffix) > 0 and len(commented_doc_lines) > 0:
commented_doc_lines.append("{}{}".format(" " * indent, suffix))
Expand Down
17 changes: 11 additions & 6 deletions src/nunavut/lang/cpp/templates/ServiceType.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,23 @@
-#}
{%- extends "base.j2" %}
{%- block object -%}
{{ T | definition_begin }}
namespace {{T|short_reference_name}}
{

{% set composite_type = T.request_type %}{% include '_composite_type.j2' %}

{% set composite_type = T.response_type %}{% include '_composite_type.j2' %}

struct {{ 'Service_%s_%s' | format(T.version.major, T.version.minor) }} {
} // namespace {{T|short_reference_name}}

{% set service_name = '%s_%s_%s' | format(T|short_reference_name, T.version.major, T.version.minor) -%}
struct {{service_name}}
{
{{service_name}}() = delete;

using Request = {{T|short_reference_name}}::{{ T.request_type | short_reference_name }};
using Response = {{T|short_reference_name}}::{{ T.response_type | short_reference_name }};

struct _traits_
{
_traits_() = delete;
Expand All @@ -21,11 +30,7 @@ struct {{ 'Service_%s_%s' | format(T.version.major, T.version.minor) }} {
static constexpr bool IsService = true;
static constexpr bool IsRequest = false;
static constexpr bool IsResponse = false;

using Request = {{ T.request_type | short_reference_name }};
using Response = {{ T.response_type | short_reference_name }};
};
};

}{{ T | definition_end }}
{%- endblock -%}
4 changes: 2 additions & 2 deletions src/nunavut/lang/cpp/templates/_composite_type.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
{% if composite_type.deprecated -%}
[[deprecated("{{ composite_type }} is reaching the end of its life; there may be a newer version available"))]]
{% endif -%}
{{ composite_type | definition_begin }} final
struct {{composite_type|short_reference_name}} final
{
struct _traits_ // The name is surrounded with underscores to avoid collisions with DSDL attributes.
{
Expand Down Expand Up @@ -127,7 +127,7 @@
{%- else -%}
{% include '_fields.j2' %}
{%- endif %}
}{{ composite_type | definition_end }}
};

{% if not nunavut.support.omit %}
inline nunavut::support::SerializeResult serialize(const {{composite_type|short_reference_name}}& obj,
Expand Down
24 changes: 12 additions & 12 deletions verification/cpp/suite/test_serialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,45 +18,45 @@ static_assert(
"Regular types are not service types");

static_assert(
regulated::basics::Service::Request_0_1::_traits_::IsServiceType,
regulated::basics::Service_0_1::Request::_traits_::IsServiceType,
"Request is a service type");
static_assert(
regulated::basics::Service::Response_0_1::_traits_::IsServiceType,
regulated::basics::Service_0_1::Response::_traits_::IsServiceType,
"Response is a service type");
static_assert(
regulated::basics::Service::Service_0_1::_traits_::IsServiceType,
regulated::basics::Service_0_1::_traits_::IsServiceType,
"Service is a service type");

static_assert(
not regulated::basics::Service::Request_0_1::_traits_::IsService,
not regulated::basics::Service_0_1::Request::_traits_::IsService,
"Request is not a service");
static_assert(
not regulated::basics::Service::Response_0_1::_traits_::IsService,
not regulated::basics::Service_0_1::Response::_traits_::IsService,
"Response is not a service");
static_assert(
regulated::basics::Service::Service_0_1::_traits_::IsService,
regulated::basics::Service_0_1::_traits_::IsService,
"Service is a service");


static_assert(
regulated::basics::Service::Request_0_1::_traits_::IsRequest,
regulated::basics::Service_0_1::Request::_traits_::IsRequest,
"Request is a request");
static_assert(
not regulated::basics::Service::Response_0_1::_traits_::IsRequest,
not regulated::basics::Service_0_1::Response::_traits_::IsRequest,
"Response is not a request");
static_assert(
not regulated::basics::Service::Service_0_1::_traits_::IsRequest,
not regulated::basics::Service_0_1::_traits_::IsRequest,
"Service is not a request");


static_assert(
not regulated::basics::Service::Request_0_1::_traits_::IsResponse,
not regulated::basics::Service_0_1::Request::_traits_::IsResponse,
"Request is not a response");
static_assert(
regulated::basics::Service::Response_0_1::_traits_::IsResponse,
regulated::basics::Service_0_1::Response::_traits_::IsResponse,
"Response is a response");
static_assert(
not regulated::basics::Service::Service_0_1::_traits_::IsResponse,
not regulated::basics::Service_0_1::_traits_::IsResponse,
"Service is not a response");


Expand Down

0 comments on commit 9dafc1c

Please sign in to comment.