[Xenomai-core] Time representation in RTDM profiles

[Xenomai-core] Time representation in RTDM profiles

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

Hi all,

the current representation of timeouts and timestamps in RTDM device
profiles is inconsistent. In the serial profile we use [u]int64_t
directly, the CAN profile defines its own types called
nanosecs_{abs|rel}_t (though they just wrap the int64 ones).

What is the idea of nanosecs? Having a way to redefine that type looks
nice, but it's unfortunately the ABI, so we cannot easily change it
without breaking apps.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

[Xenomai-core] Re: Time representation in RTDM profiles

[Sebastian Smolorz]

Jan Kiszka wrote:
> the current representation of timeouts and timestamps in RTDM device
> profiles is inconsistent.

I would welcome a consistent time value representation above all RTDM 
profiles, too.

> In the serial profile we use [u]int64_t 
> directly, the CAN profile defines its own types called
> nanosecs_{abs|rel}_t (though they just wrap the int64 ones).
>
> What is the idea of nanosecs? Having a way to redefine that type looks
> nice, but it's unfortunately the ABI, so we cannot easily change it
> without breaking apps.

The possibility of redefinition was not the main goal here. As you mentioned 
it would be problematic. No, I introduced nanosecs_abs_t and nanosecs_rel_t 
because they are more intuitive and more "speaking" to the programmer. The 
meaning of a variable of such a type is clear at first sight.

Sebastian

[Xenomai-core] Re: Time representation in RTDM profiles

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

Sebastian Smolorz wrote:
> Jan Kiszka wrote:
>> the current representation of timeouts and timestamps in RTDM device
>> profiles is inconsistent.
> 
> I would welcome a consistent time value representation above all RTDM 
> profiles, too.
> 
>> In the serial profile we use [u]int64_t 
>> directly, the CAN profile defines its own types called
>> nanosecs_{abs|rel}_t (though they just wrap the int64 ones).
>>
>> What is the idea of nanosecs? Having a way to redefine that type looks
>> nice, but it's unfortunately the ABI, so we cannot easily change it
>> without breaking apps.
> 
> The possibility of redefinition was not the main goal here. As you mentioned 
> it would be problematic. No, I introduced nanosecs_abs_t and nanosecs_rel_t 
> because they are more intuitive and more "speaking" to the programmer. The 
> meaning of a variable of such a type is clear at first sight.
> 

Yeah, sounds reasonable to me. Then let's move these typedefs to rtdm.h
and document them as self-explanatory defines of the underlying standard
types, freezing their width and signedness at the same time.

Actually, this would be useful for the driver API of RTDM as well.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

[Xenomai-core] Re: Time representation in RTDM profiles

[Sebastian Smolorz]

Jan Kiszka wrote:
> Sebastian Smolorz wrote:
> > The possibility of redefinition was not the main goal here. As you
> > mentioned it would be problematic. No, I introduced nanosecs_abs_t and
> > nanosecs_rel_t because they are more intuitive and more "speaking" to the
> > programmer. The meaning of a variable of such a type is clear at first
> > sight.
>
> Yeah, sounds reasonable to me. Then let's move these typedefs to rtdm.h
> and document them as self-explanatory defines of the underlying standard
> types, freezing their width and signedness at the same time.

Agreed.

>
> Actually, this would be useful for the driver API of RTDM as well.

That's right. No objections from my side.

Sebastian

[Xenomai-core] Re: Time representation in RTDM profiles

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

Sebastian Smolorz wrote:
> Jan Kiszka wrote:
>> Sebastian Smolorz wrote:
>>> The possibility of redefinition was not the main goal here. As you
>>> mentioned it would be problematic. No, I introduced nanosecs_abs_t and
>>> nanosecs_rel_t because they are more intuitive and more "speaking" to the
>>> programmer. The meaning of a variable of such a type is clear at first
>>> sight.
>> Yeah, sounds reasonable to me. Then let's move these typedefs to rtdm.h
>> and document them as self-explanatory defines of the underlying standard
>> types, freezing their width and signedness at the same time.
> 
> Agreed.
> 
>> Actually, this would be useful for the driver API of RTDM as well.
> 
> That's right. No objections from my side.
> 

Done.

Though I checked things more then twice, some regression may be hidden,
specifically as I changed the signedness of timeout parameters of a few
RTDM driver API functions. All documented under
ksrc/skins/rtdm/API.CHANGES, and everyone is warned now.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

[Xenomai-core] Re: Time representation in RTDM profiles

[Sebastian Smolorz]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

Jan Kiszka wrote:
> Sebastian Smolorz wrote:
> > Jan Kiszka wrote:
> >> Sebastian Smolorz wrote:
> >>> The possibility of redefinition was not the main goal here. As you
> >>> mentioned it would be problematic. No, I introduced nanosecs_abs_t and
> >>> nanosecs_rel_t because they are more intuitive and more "speaking" to
> >>> the programmer. The meaning of a variable of such a type is clear at
> >>> first sight.
> >>
> >> Yeah, sounds reasonable to me. Then let's move these typedefs to rtdm.h
> >> and document them as self-explanatory defines of the underlying standard
> >> types, freezing their width and signedness at the same time.
> >
> > Agreed.
> >
> >> Actually, this would be useful for the driver API of RTDM as well.
> >
> > That's right. No objections from my side.
>
> Done.
>
> Though I checked things more then twice, some regression may be hidden,
> specifically as I changed the signedness of timeout parameters of a few
> RTDM driver API functions. All documented under
> ksrc/skins/rtdm/API.CHANGES, and everyone is warned now.

Here comes a follow-up patch. Compile-tested successfully.

Sebastian

[-- Attachment #2: rtcan_timeout.patch --]
[-- Type: text/x-diff, Size: 3886 bytes --]

Index: include/rtdm/rtcan.h
===================================================================
--- include/rtdm/rtcan.h	(Revision 1450)
+++ include/rtdm/rtcan.h	(Arbeitskopie)
@@ -507,16 +507,6 @@
 /** @} */
 
 
-/*!
- * @anchor RTCAN_TIMEOUTS   @name Timeouts
- * Special reception and transmission timeouts
- * @{ */
-#define RTCAN_TIMEOUT_INFINITE    0     /**< wait for ever  */
-#define RTCAN_TIMEOUT_NONE        (-1)  /**< equivalent to non-blocking */
-/** @} */
-
-
-
 #define RTIOC_TYPE_CAN              RTDM_CLASS_CAN
 
 
@@ -946,7 +936,7 @@
  * @param [in] arg Pointer to @ref nanosecs_rel_t variable. The value is
  *                interpreted as relative timeout in nanoseconds in case
  *                of a positive value.
- *                See @ref RTCAN_TIMEOUTS "Timeouts" for special timeouts.
+ *                See @ref RTDM_TIMEOUT_xxx "Timeouts" for special timeouts.
  *
  * @return 0 on success, otherwise:
  * - -EFAULT: It was not possible to access user space memory area at the
@@ -976,7 +966,7 @@
  * @param [in] arg Pointer to @ref nanosecs_rel_t variable. The value is
  *                interpreted as relative timeout in nanoseconds in case
  *                of a positive value.
- *                See @ref RTCAN_TIMEOUTS "Timeouts" for special timeouts.
+ *                See @ref RTDM_TIMEOUT_xxx "Timeouts" for special timeouts.
  *
  * @return 0 on success, otherwise:
  * - -EFAULT: It was not possible to access user space memory area at the
Index: ChangeLog
===================================================================
--- ChangeLog	(Revision 1450)
+++ ChangeLog	(Arbeitskopie)
@@ -1,3 +1,8 @@
+2006-08-16  Sebastian Smolorz  <Sebastian.Smolorz@domain.hid>
+
+	* include/rtdm/rtcan.h, ksrc/drivers/can/rtcan_{module,socket,raw}.c:
+	Get rid of RTCAN_TIMEOUT_* defines and replace them with RTDM_TIMEOUT_*
+
 2006-08-16  Jan Kiszka  <jan.kiszka@domain.hid>
 
 	* include/rtdm/*.h, ksrc/skins/rtdm/drvlib.c: Refactor RTDM types
Index: ksrc/drivers/can/rtcan_module.c
===================================================================
--- ksrc/drivers/can/rtcan_module.c	(Revision 1450)
+++ ksrc/drivers/can/rtcan_module.c	(Arbeitskopie)
@@ -112,7 +112,7 @@
 static void rtcan_get_timeout_name(nanosecs_rel_t timeout,
 				   char* name, int max_len)
 {
-    if (timeout == RTCAN_TIMEOUT_INFINITE) 
+    if (timeout == RTDM_TIMEOUT_INFINITE) 
 	strncpy(name, "infinite", max_len);
     else
 	sprintf(name, "%lld", timeout);
Index: ksrc/drivers/can/rtcan_socket.c
===================================================================
--- ksrc/drivers/can/rtcan_socket.c	(Revision 1450)
+++ ksrc/drivers/can/rtcan_socket.c	(Arbeitskopie)
@@ -49,8 +49,8 @@
     sock->err_mask = 0;
     sock->rx_buf_full = 0;
 
-    sock->tx_timeout = RTCAN_TIMEOUT_INFINITE;
-    sock->rx_timeout = RTCAN_TIMEOUT_INFINITE;
+    sock->tx_timeout = RTDM_TIMEOUT_INFINITE;
+    sock->rx_timeout = RTDM_TIMEOUT_INFINITE;
 
     INIT_LIST_HEAD(&sock->tx_wait_head);
     list_add(&sock->socket_list, &rtcan_socket_list);
Index: ksrc/drivers/can/rtcan_raw.c
===================================================================
--- ksrc/drivers/can/rtcan_raw.c	(Revision 1450)
+++ ksrc/drivers/can/rtcan_raw.c	(Arbeitskopie)
@@ -556,7 +556,7 @@
     /* Set RX timeout */
     rtdm_lock_get_irqsave(&rtcan_socket_lock, lock_ctx);
 
-    timeout = (flags & MSG_DONTWAIT) ? RTCAN_TIMEOUT_NONE : sock->rx_timeout;
+    timeout = (flags & MSG_DONTWAIT) ? RTDM_TIMEOUT_NONE : sock->rx_timeout;
 
     rtdm_lock_put_irqrestore(&rtcan_socket_lock, lock_ctx);
 
@@ -839,7 +839,7 @@
    
     rtdm_lock_get_irqsave(&rtcan_socket_lock, lock_ctx);
 
-    timeout = (flags & MSG_DONTWAIT) ? RTCAN_TIMEOUT_NONE : sock->tx_timeout;
+    timeout = (flags & MSG_DONTWAIT) ? RTDM_TIMEOUT_NONE : sock->tx_timeout;
 
     rtdm_lock_put_irqrestore(&rtcan_socket_lock, lock_ctx);
 

Re: [Xenomai-core] Re: Time representation in RTDM profiles

[Jan Kiszka]

Sebastian Smolorz wrote:
> Jan Kiszka wrote:
>> Sebastian Smolorz wrote:
>>> Jan Kiszka wrote:
>>>> Sebastian Smolorz wrote:
>>>>> The possibility of redefinition was not the main goal here. As you
>>>>> mentioned it would be problematic. No, I introduced nanosecs_abs_t and
>>>>> nanosecs_rel_t because they are more intuitive and more "speaking" to
>>>>> the programmer. The meaning of a variable of such a type is clear at
>>>>> first sight.
>>>> Yeah, sounds reasonable to me. Then let's move these typedefs to rtdm.h
>>>> and document them as self-explanatory defines of the underlying standard
>>>> types, freezing their width and signedness at the same time.
>>> Agreed.
>>>
>>>> Actually, this would be useful for the driver API of RTDM as well.
>>> That's right. No objections from my side.
>> Done.
>>
>> Though I checked things more then twice, some regression may be hidden,
>> specifically as I changed the signedness of timeout parameters of a few
>> RTDM driver API functions. All documented under
>> ksrc/skins/rtdm/API.CHANGES, and everyone is warned now.
> 
> Here comes a follow-up patch. Compile-tested successfully.
> 

Just applied, thanks.

[I thought about dropping the RTSER_TIMEOUT_* stuff as well, but A)
there are already users and B) it aligns so well with the other defines
for rtser_config.]

Jan