From Dr. Mizukawa,

device_kind(Table 7.8: Device) is same as Device Kind ID?

From Dr. Mizukawa,

7.4.5.1 HAL Component
profileId
A Profile ID identifies the kind of the device.

Is Profile ID same as Device Kind ID?

From Matthew Hause,

It concerns me that there is no mechanism included for managing the ID for devices. Without this mechanism, it will be difficult for the implementation to succeed. The specification should at least describe how this is planned to be accomplished.

From Matthew Hause,

Many of the definitions in the document are tautologies. In other words, the definition of the element simply repeats the name of the element. I will not list all of them, but this is the case for most definitions. For example:
“7.4.5.14 Direction Sensor
Direction Sensor is a sensor to measure direction.”
In some cases this is unavoidable, but in other cases the terms may not be easily understandable. This needs to be fixed in the document.

More condition for conformance is needed.

More definition for UML-to-C Transformation needed to explain mapping between PIM(7.4) and C PSM(7.5.2).

Init() function must have some arguments including device ID but no argument in the specification.
And also Init() function must returns device (HAL component's) handle or device descriptor.

completed(), notify_error() and some other callback functions exist in the specification. These callback functions must be bound to device layer or more deeper layer device entity. However binding mechanism and its related functions are not described in the specification.

Callback functions binding mechanism should be provided and described.

In the definition of interfaces of Surface Layer, that are implemented on Surface App, both a set of methods for Application Base and another set of methods for Device Layer are defined in a same interface.
For example, in the definition of System Interface, ‘get_error_detail()’ should be exposed only to Application Base, on the other hand, ’notify_error()’ should be exposed only to Device Layer.
Those methods should be defined in two different interfaces to avoid inappropriate exposure.

It isn't a sensor.

Unclear about DOP, Signal Strength etc.

In Device layer, there are few attributes for robotic system.

The definition for Device Kind ID Registry is poor.

Why does the operation, "SetSpeed" have the argument, tmAcc (Acceleration/Deceleration time to the target speed)?

About attributes for Motor, why the word, "torque" is used for motor? Not "force".

There are 2 typos.

incorrect : Set the current position as origin
correct : Set the offset position as origin

incorrect : The target position
correct : The offset position

In places in the specification the Octet type from the RTC specification is used. For example, in 7.4.5.1 "HAL Component". Additionally, this type is then mapped by 7.5.1.1.2 to a 32-bit signed integer, which is not what is implied by the name "octet" (i.e. a byte). Instead of pulling in the RTC specification for an octet type that is not used correctly, a "byte" should be specified, which is then mapped to uint8_t in the C PSM.

dtc-16-01-01 section 7.5.2 C PSM and robotics/15-11-04 are a header of HAL's surface layer.
Most of functions declared in there have no arguments.

For example completed() function is declared in the header as follows,

int32_t completed();

However, in the Table 7.5 section 7.4.3.1 in page 21, completed() function is defined as follows.

ReturnCode completed(string command_id, ReturnCode status);

The C PSM given in 7.5.2 uses int32_t and uint32_t. Unless there is a specific requirement for these values to be 32 bits, they should be specified as "int" and "unsigned int" and it be left up to the compiler for the platform in use to choose appropriate sizes.

From Matthew Hause,

7.4.5.14 Direction Sensor
Date is a Real with range from 2015.0 to 2020.0. The date needs to extend past 2020 or the specification will quickly become obsolete.

From Matthew Hause,

Figure 7.10 – State Machine of Motor
The details of each state are below.
・Wait(Not Excited)：
Is the word “exited” intended instead?

From Matthew Hause,

7.4.5.1 HAL Component
What is meant by Octet [RTC]? Some of the attributes are typed by this.

From Matthew Hause,

Section 7.4.5
“If the nature of the product, an API that does not function exists, it will implement all of the API.”
Please clarify this sentence as it is confusing.
Same for “(A motor which is corresponding to only the speed control in, or if it can not support the torque control API · Position Control API) And, non-support API will be implemented as you plan to return the "API unimplemented error (HAL_NOT_IMPLEMENTED)" as the API of the return value.”

From Matthew Hause,

Section 7.4.4 Table 7.8: Device
The kind of the device. For examples, Motor, Encoder, Gyro sensor, Acceleration sensor, Force sensor, Torque sensor, Temperature sensor, Humidity sensor, GPS sensor, Direction sensor etc.
device_kind is a string. Does this mean that you will be using character matching to find the name of the device? Would it be better to use an enum or other type of construct?

From Matthew hause,

If the return codes are an enum, why are they typed as a string and not as the enum or a short or int?

Since HAL4RT_Surface.h (robotics/15-11-04) includes some syntax errors, it cannot be compiled.
1. To use int32_t #include <stdint.h> directive necessary. (C99)
2. Invalid function pointers declarations in SENSOR structure.
3. Invalid #endif syntax.

At least the following modifications are necessary.

--- org/HAL4RT_Surface.h        2016-03-15 03:11:51.496000000 +0900
+++ mod/HAL4RT_Surface.h        2016-03-15 03:27:33.664000000 +0900
@@ -1,6 +1,7 @@
 #ifndef HAL4RT_SURFACE_H
 #define HAL4RT_SURFACE_H

+#include <stdint.h>

 int32_t completed();
 int32_t notify_event();
@@ -14,30 +15,30 @@


 typedef struct ApplicationBase {
-  int32_t completed();
-  int32_t notify_event();
-  int32_t notify_error();
+  int32_t (*completed)();
+  int32_t (*notify_event)();
+  int32_t (*notify_error)();
 } APPLICATIONBASE;


 typedef struct System {
-  int32_t notify_error();
-  int32_t get_error_detail();
+  int32_t (*notify_error)();
+  int32_t (*get_error_detail)();
 } SYSTEM;


 typedef struct Command {
-  int32_t execute();
-  int32_t completed();
+  int32_t (*execute)();
+  int32_t (*completed)();
 } COMMAND;


 typedef struct Event {
-  int32_t subscribe();
-  int32_t unsubscribe();
-  int32_t notify_event();
-  int32_t get_event_data();
+  int32_t (*subscribe)();
+  int32_t (*unsubscribe)();
+  int32_t (*notify_event)();
+  int32_t (*get_event_data)();
 } EVENT;


-#endif HAL4RT_SURFACE_H
+#endif /* HAL4RT_SURFACE_H */

From Matthew Hause,

Section 7.4.1
“[OPTIONAL] In addition, the Surface layer is not required because HAL4RT aims “light and compact” specification.”
I am not sure what this statement means. It seems to say that the Surface layer is not necessary. However, the previous text is all about describing why it is necessary. Please explain.

Since HAL4RT_Device.h (robotics/15-11-05) includes some syntax errors, it cannot be compiled.

1. To use int32_t #include <stdint.h> directive necessary. (C99)
2.Syntax errors without sentence termination character ";" in ACTUATOR and SENSOR structure declarations.
3. Invalid function pointers declarations in SENSOR structure.
4. Invalid #endif syntax.

--- org/HAL4RT_Device.h 2016-03-15 03:11:51.496000000 +0900
+++ mod/HAL4RT_Device.h 2016-03-15 03:14:42.412000000 +0900
@@ -1,6 +1,7 @@
 #ifndef HAL4RT_DEVICE_H
 #define HAL4RT_DEVICE_H

+#include <stdint.h>

 typedef struct HALComponent {
   char ComponentName[32];
@@ -13,7 +14,7 @@

 typedef struct Actuator {
   uint32_t ActuatorKindId;
-} ACTUATOR
+} ACTUATOR;


 typedef struct Sensor {
@@ -21,13 +22,13 @@
   int32_t ValueOfMeasurement[32];
   int32_t ScaleFactor[32];
   int32_t Origin[32];
-  int32_t GetSensorKind();
-  int32_t GetValueOfMeasurement();
-  int32_t SetUintOfMeasurement();
-  int32_t GetUnitOfMeasurement();
-  int32_t SetScaleFactor();
-  int32_t SetOrigin();
-} SENSOR
+  int32_t (*GetSensorKind)();
+  int32_t (*GetValueOfMeasurement)();
+  int32_t (*SetUintOfMeasurement)();
+  int32_t (*GetUnitOfMeasurement)();
+  int32_t (*SetScaleFactor)();
+  int32_t (*SetOrigin)();
+} SENSOR;


-#endif HAL4RT_DEVICE_H
+#endif /* HAL4RT_DEVICE_H */

The definition of notify_error() functions in "ApplicationBase" and "System" are inconsistent.

p.19, 7.4.3.1, Table 7.4
notify_error()'s return type is not defined explicitly, but definition of class and interface table in 7.2 describes as follows: "Also, as the type of return code for every operation in this specification is Returncode, which is defined in Section7.3, Return Codes, this is omitted in the description table." Therefore, return type of the notify_error() is ReturnCode from the definition in the table.

p.19, 7.4.3, Figure 7.6
ApplicationBase::notify_error() and System::notify_error() in the figure are defined as having no return value.

p.21, 7.4.3.2, Table 7.7
Return type of the notify_error() is ReturnCode from the definition in the table.

p.33, 7.5.2 C PSM
ApplicationBase::notify_error(), Event::notify_error() are defined as having int32_t type return value.

Since the notify_error() functions are just callback functions, return value of "notify_error" seems not necessary.

The definition of notify_event() functions in "ApplicationBase" and "Event" are inconsistent.

p.19, 7.4.3, Figure 7.6
ApplicationBase::notify_event() is defined as having int type return value.

p.20, 7.4.3.1, Table 7.6
notify_event()'s return type is not defined explicitly, but definition of class and interface table in 7.2 describes as follows: "Also, as the type of return code for every operation in this specification is Returncode, which is defined in Section7.3, Return Codes, this is omitted in the description table."
Therefore, return type of the notify_event() is ReturnCode from the definition in the table.

p.21, 7.4.3.2, Table 7.7
Return type of the notify_event() is ReturnCode from the definition in the table.

p.23, 7.4.5, Figure 7.8
Event::notify_event() is defined as no retunr value.

p.33, 7.5.2 C PSM
ApplicationBase::notify_event(), Event::notify_event() are defined as habing int32_t type return value.

Since the notify_event() functions are just callback functions, return value of "notify_event" seems not necessary.

Some notation variability of terms: Application Program(ming) Interface exist.

p.13,7.1 General
Specifically, HAL4RT is an API (Application Program Interface) for the layer between on

p.15, 7.4.1 Overview
The Surface layer provides standardized API (Application Program Interface) to application software

p.19, 7.4.3 Surface layer
The surface layer is the specification of the Application Programming Interface for the

"HAL4RT components" first appeared without any description in the section 7.4.1.
It would be better add a reference to "7.4.5.1 HAL component."

In the section 7.3, table 7.3, six return codes HAL_(OK, NO_CONNECTED, NO_MEMORY, NULL_PARAMETER, NOT_IMPLEMENTED, BAD_PARAMETER) are defined. Error codes is too specific to describe generic erros.

For example, a device might returns device specific error such as "Invalid initialization error", "Motor is too hot", "Sensor broken" and so on. If device returns these kind of errors, current definition cannot notify these kind of error.

A generic error code such as HAL_ERROR should be defined for user/device vendor dependent error and/or generic error.

In the section 7.3, some return codes are defined.

The meaning of HAL_NULL_PARAMETER is described as "The parameter is not supported." The meaning of HAL_BAD_PARAMETER is described as "The operation failed because an illegal argument was passed to it."

The difference between "unsupported parameter" and "illegal arguments" are ambiguous, and usage of return codes are not defined clearly in the following specification description. This definition might confuse implementors.

In the section 7.3 Return Codes, the return code type and some return codes are defined in Table 7.3. One of the return code "HAL_NO_CONNECTED" naming is not grammatically correct. It should be "HAL_NO_CONNECTION" or "HAL_NOT_CONNECTED."