[PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines

[PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines

[Aldrin Martoq]

Hey hackers,

While developing pyalsa sequencer API, I found that sometimes 0 is
used instead of a #defined value or constant.

In my opinion, given that the alsa API is opaque (hides internal
structure and provides methods for accessing them), we should define
those missing "constants" for being consistent. So far I found:
SND_SEQ_BLOCK 0 (blocking mode)
SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)



BTW, Thanks Jaroslav for putting my code; I will send 3rd patch for
covering the missing items of the python API sequencer when I'll
finish it.

Signed-off-by: Aldrin Martoq <amartoq@dcc.uchile.cl>
---
diff --git a/include/seq.h b/include/seq.h
--- a/include/seq.h
+++ b/include/seq.h
@@ -56,7 +56,8 @@ typedef struct _snd_seq snd_seq_t;
 /**
  * sequencer opening mode
  */
-#define SND_SEQ_NONBLOCK       0x0001  /**< non-blocking mode (flag
to open mode) */
+#define SND_SEQ_BLOCK          0x0000  /**< blocking opening mode
(flag for #snd_seq_open, #snd_seq_nonblock) */
+#define SND_SEQ_NONBLOCK       0x0001  /**< non-blocking opening mode
(flag for #snd_seq_open, #snd_seq_nonblock) */

 /** sequencer handle type */
 typedef enum _snd_seq_type {
@@ -204,6 +205,7 @@ typedef struct _snd_seq_port_info snd_se
 #define SND_SEQ_PORT_SYSTEM_ANNOUNCE   1       /**< system announce port */

 /** port capabilities (32 bits) */
+#define SND_SEQ_PORT_CAP_NONE          (0<<0)  /**< port has no
access capabilities */
 #define SND_SEQ_PORT_CAP_READ          (1<<0)  /**< readable from this port */
 #define SND_SEQ_PORT_CAP_WRITE         (1<<1)  /**< writable to this port */

diff --git a/src/seq/seq.c b/src/seq/seq.c
--- a/src/seq/seq.c
+++ b/src/seq/seq.c
@@ -958,7 +958,7 @@ static int snd_seq_open_noupdate(snd_seq
  * \note Internally, these are translated to \c O_WRONLY, \c O_RDONLY and
  * \c O_RDWR respectively and used as the second argument to the C library
  * open() call.
- * \param mode Optional modifier.  Can be either 0, or
+ * \param mode Optional modifier.  Can be either #SND_SEQ_BLOCK, or
  * #SND_SEQ_NONBLOCK, which will make read/write operations
  * non-blocking.  This can also be set later using #snd_seq_nonblock().
  * \return 0 on success otherwise a negative error code
@@ -2161,8 +2161,9 @@ void snd_seq_port_info_set_timestamp_que
  * Each port has the capability bit-masks to specify the access capability
  * of the port from other clients.
  * The capability bit flags are defined as follows:
+ * - #SND_SEQ_PORT_CAP_NONE No access capabilities
  * - #SND_SEQ_PORT_CAP_READ Readable from this port
- * - #SND_SEQ_PORT_CAP_WRITE Writable to this port.
+ * - #SND_SEQ_PORT_CAP_WRITE Writable to this port
  * - #SND_SEQ_PORT_CAP_SYNC_READ For synchronization (not implemented)
  * - #SND_SEQ_PORT_CAP_SYNC_WRITE For synchronization (not implemented)
  * - #SND_SEQ_PORT_CAP_DUPLEX Read/write duplex access is supported



-- 
Aldrin Martoq

Re: [PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines

[Takashi Iwai]

At Thu, 31 Jan 2008 14:42:42 -0300,
Aldrin Martoq wrote:
> 
> Hey hackers,
> 
> While developing pyalsa sequencer API, I found that sometimes 0 is
> used instead of a #defined value or constant.
> 
> In my opinion, given that the alsa API is opaque (hides internal
> structure and provides methods for accessing them), we should define
> those missing "constants" for being consistent. So far I found:
> SND_SEQ_BLOCK 0 (blocking mode)
> SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)

Well, I myself don't mind to add such defintions, but remember that
these flags are no enum but bits.  When you see BLOCK and NONBLOCK
definitions, you'll likely think these are exclusive.  However,
(BLOCK|NONBLOCK) or (NONE|READ) can be passed without problems.


Takashi

> 
> 
> 
> BTW, Thanks Jaroslav for putting my code; I will send 3rd patch for
> covering the missing items of the python API sequencer when I'll
> finish it.
> 
> Signed-off-by: Aldrin Martoq <amartoq@dcc.uchile.cl>
> ---
> diff --git a/include/seq.h b/include/seq.h
> --- a/include/seq.h
> +++ b/include/seq.h
> @@ -56,7 +56,8 @@ typedef struct _snd_seq snd_seq_t;
>  /**
>   * sequencer opening mode
>   */
> -#define SND_SEQ_NONBLOCK       0x0001  /**< non-blocking mode (flag
> to open mode) */
> +#define SND_SEQ_BLOCK          0x0000  /**< blocking opening mode
> (flag for #snd_seq_open, #snd_seq_nonblock) */
> +#define SND_SEQ_NONBLOCK       0x0001  /**< non-blocking opening mode
> (flag for #snd_seq_open, #snd_seq_nonblock) */
> 
>  /** sequencer handle type */
>  typedef enum _snd_seq_type {
> @@ -204,6 +205,7 @@ typedef struct _snd_seq_port_info snd_se
>  #define SND_SEQ_PORT_SYSTEM_ANNOUNCE   1       /**< system announce port */
> 
>  /** port capabilities (32 bits) */
> +#define SND_SEQ_PORT_CAP_NONE          (0<<0)  /**< port has no
> access capabilities */
>  #define SND_SEQ_PORT_CAP_READ          (1<<0)  /**< readable from this port */
>  #define SND_SEQ_PORT_CAP_WRITE         (1<<1)  /**< writable to this port */
> 
> diff --git a/src/seq/seq.c b/src/seq/seq.c
> --- a/src/seq/seq.c
> +++ b/src/seq/seq.c
> @@ -958,7 +958,7 @@ static int snd_seq_open_noupdate(snd_seq
>   * \note Internally, these are translated to \c O_WRONLY, \c O_RDONLY and
>   * \c O_RDWR respectively and used as the second argument to the C library
>   * open() call.
> - * \param mode Optional modifier.  Can be either 0, or
> + * \param mode Optional modifier.  Can be either #SND_SEQ_BLOCK, or
>   * #SND_SEQ_NONBLOCK, which will make read/write operations
>   * non-blocking.  This can also be set later using #snd_seq_nonblock().
>   * \return 0 on success otherwise a negative error code
> @@ -2161,8 +2161,9 @@ void snd_seq_port_info_set_timestamp_que
>   * Each port has the capability bit-masks to specify the access capability
>   * of the port from other clients.
>   * The capability bit flags are defined as follows:
> + * - #SND_SEQ_PORT_CAP_NONE No access capabilities
>   * - #SND_SEQ_PORT_CAP_READ Readable from this port
> - * - #SND_SEQ_PORT_CAP_WRITE Writable to this port.
> + * - #SND_SEQ_PORT_CAP_WRITE Writable to this port
>   * - #SND_SEQ_PORT_CAP_SYNC_READ For synchronization (not implemented)
>   * - #SND_SEQ_PORT_CAP_SYNC_WRITE For synchronization (not implemented)
>   * - #SND_SEQ_PORT_CAP_DUPLEX Read/write duplex access is supported
> 
> 
> 
> -- 
> Aldrin Martoq
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines

[Aldrin Martoq]

On Fri, 2008-02-01 at 13:38 +0100, Takashi Iwai wrote:
> At Thu, 31 Jan 2008 14:42:42 -0300,
> Aldrin Martoq wrote:
> > Hey hackers,
> > While developing pyalsa sequencer API, I found that sometimes 0 is
> > used instead of a #defined value or constant.
> > In my opinion, given that the alsa API is opaque (hides internal
> > structure and provides methods for accessing them), we should define
> > those missing "constants" for being consistent. So far I found:
> > SND_SEQ_BLOCK 0 (blocking mode)
> > SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
> Well, I myself don't mind to add such defintions, but remember that
> these flags are no enum but bits.  When you see BLOCK and NONBLOCK
> definitions, you'll likely think these are exclusive.  However,
> (BLOCK|NONBLOCK) or (NONE|READ) can be passed without problems.

I'm sorry, you are right and I was absolutely wrong. I thought mode is
something like an enumeration rather than a flag (I think still there is
a problem, for example there is no "change mode" api function, instead
there is a snd_seq_nonblock function ?!); the port cap type was clearly
a mistake.

I will correct this in the python version (remove this non-meaning
constants) and please forget my patch. Thanks,

-- 
Aldrin Martoq <amartoq@dcc.uchile.cl>

Re: [PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines

[Jaroslav Kysela]

On Thu, 31 Jan 2008, Aldrin Martoq wrote:

> Hey hackers,
> 
> While developing pyalsa sequencer API, I found that sometimes 0 is
> used instead of a #defined value or constant.
> 
> In my opinion, given that the alsa API is opaque (hides internal
> structure and provides methods for accessing them), we should define
> those missing "constants" for being consistent. So far I found:
> SND_SEQ_BLOCK 0 (blocking mode)
> SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)

Note that SND_SEQ_NONBLOCK is flag (bitmask). Also SND_SEQ_PORT_CAP_* 
defines are bitmasks. I don't see usage to have defined for 0 because it 
always mean "off state" rather than a direct value. Correct tests are:

val&SND_SEQ_NONBLOCK		- nonblocking behaviour
(val&SND_SEQ_NONBLOCK)==0	- blocking behaviour

Regarding python. I think that we need to settle naming scheme. I just 
moved my code to follow standard python rules (only class names have upper 
letters and constants).

I also chose a different organization for constants in my code to have 
possibility to enumerate all related bitmasks or numbers using 
dictionaries. See my python modules.

It meas to define SEQ_ADDRESS_* like:

seq_address = {'BROADCAST': 0xff, 'SUBSCRIBERS': 0xfe, 'UNKNOWN': 0xfd}
open_mode = {'NONBLOCK': 1}
etc...

				Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] SND_SEQ_BLOCK and SND_SEQ_PORT_CAP_NONE defines

[Aldrin Martoq]

On Wed, 2008-02-06 at 10:51 +0100, Jaroslav Kysela wrote:
> On Thu, 31 Jan 2008, Aldrin Martoq wrote:
> > While developing pyalsa sequencer API, I found that sometimes 0 is
> > used instead of a #defined value or constant.
> > In my opinion, given that the alsa API is opaque (hides internal
> > structure and provides methods for accessing them), we should define
> > those missing "constants" for being consistent. So far I found:
> > SND_SEQ_BLOCK 0 (blocking mode)
> > SND_SEQ_PORT_CAP_NONE (the port defines no access capabilites)
> Note that SND_SEQ_NONBLOCK is flag (bitmask). Also SND_SEQ_PORT_CAP_* 
> defines are bitmasks. I don't see usage to have defined for 0 because it 
> always mean "off state" rather than a direct value. Correct tests are:
> val&SND_SEQ_NONBLOCK		- nonblocking behaviour
> (val&SND_SEQ_NONBLOCK)==0	- blocking behaviour

Yup, I was sorry for my mistake; I'm fixing (removing) this in my
current python binding.

I think my mistake originated because there is no snd_seq_set_mode
function API. The snd_seq_nonblock method is inconsistent with the flag
behaivor. So, I'm sticking with changing the mode of a Sequencer
instance through Sequencer.mode attribute.

> Regarding python. I think that we need to settle naming scheme. I just 
> moved my code to follow standard python rules (only class names have upper 
> letters and constants).

Yeah, I think we should follow current python PEP's:
a) Naming conventions
http://www.python.org/dev/peps/pep-0008/
b) Doc strings
http://www.python.org/dev/peps/pep-0257/

a) Says basically:
- Class names are CapsWords; like in SeqEvent or SequencerError.
- method names are lowercase and _ separated, like in
get_mixer_controls().
- members of classes lowercase and _ separated.
- There are no strict rules, so this are recommended practices.
Sometimes, some modules follow other rules (ex: st_mode).

However, I couldn't find anything regarding constant naming in PEPs. I
suggest:
- keeping names all uppercase and _ separated. Must follow or imitate
original C naming; like in SND_SEQ_NONBLOCK.
- I'm not sure if keeping constants "inside" the module or make them
exportable ("from alsaseq import *"). In the first way, we may follow
python-gtk naming scheme (ex: gtk.gdk.AXIS_IGNORE); in the second way,
we may stick to something like alsaseq.SND_SEQ_NONBLOCK or
alsaseq.SEQ_NONBLOCK.

http://www.pygtk.org/docs/pygtk/gdk-constants.html


Jaroslav, What do you prefer?



> I also chose a different organization for constants in my code to have 
> possibility to enumerate all related bitmasks or numbers using 
> dictionaries. See my python modules.
> It meas to define SEQ_ADDRESS_* like:
> seq_address = {'BROADCAST': 0xff, 'SUBSCRIBERS': 0xfe, 'UNKNOWN': 0xfd}
> open_mode = {'NONBLOCK': 1}
> etc...

I like your idea, and I'm thinking to change to that.

I first created the Constant class to force use them instead of an
integer... The method was, raise a Warning error if the user didn't
provide a Constant instance for some argument. That's why I implemented
the NumProtocol for Constant: it returns a Constant for a bitwise
between Constants... Then I realised this was too much strict type
checking (like java) and not the python way, so I'm thinking of removing
the numeric protocol for Constant. Note that the current code doesn't
throw any warning.

However, I think a Constant class is useful, if you call __repr__ or
__str__ to an instance it will return the name instead of looking
through a dictionary.



Thanks for all your help, I'm still working for completing the full
sequencer API. I'll be done/happy if a cover 80% of it: I've realized
that open_lconf will need a new python binding for the Configuration
API.

-- 
Aldrin Martoq <amartoq@dcc.uchile.cl>