This MIDI stuff, would someone be interested in reviewing my code?

This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

Hi

It's still early days but, if I'm going to be using Ada to try to
build this app I want, it would be nice to write it in a style that
looks appropriate. I'm aware of the Q&S guide but I was hoping that
someone could take a quick look at the code I've written (it's only 80
lines or so, and it's down below) and see if there's anything
obviously stupid I'm doing.

My specific thoughts on this are:

1) Perhaps I should be using limited withs in some places to get
access to the primitive operators/functions of the stuff in
Interfaces.C/.strings and Win32 etc.

2) The for loops: for devId in Win32.UINT range 0..(NumOutputDevices -
1) etc. These are protected by a "if NumOutputDevices < 0" condition
but before I realised my mistake I found that when NumOutputDevices is
0, the loop executes as many times as it can before it crashed. This
was obviously because NumOutputDevices was 0, so the range
"0..(NumOutputDevices - 1)" was 0..4929blahblah due to Win32.UINT
being a modular type. I looked at the option to use something like:
   for index in Win32.UINT range 1..NumOuputDevices loop
      declare
         devId : Win32.UINT := index - 1;
      begin
         ...
      end;
   end loop;
but stuck with the original with the conditional round it.

3) Would it be more appropriate to use something like
Win32.UINT'Image() instead of getting an instantiation of the
Modular_IO package?

Anyway - thanks to anyone who can be bothered to look at this. It will
be much appreciated, and thanks for everyone's help so far.

John


===================
with Ada.Text_IO;
with Ada.Unchecked_Deallocation;

with Interfaces.C; use Interfaces.C;
with Interfaces.C.Strings; use Interfaces.C.Strings;

with Win32; use Win32;
with Win32.Mmsystem; use Win32.Mmsystem;

procedure MidiDevs is
    NumInputDevices  : Win32.UINT;
    NumOutputDevices : Win32.UINT;

    res         : Win32.Mmsystem.MMRESULT;
    midiInCaps  : Win32.Mmsystem.LPMIDIINCAPS;
    midiOutCaps : Win32.Mmsystem.LPMIDIOUTCAPS;

    package UINTText_IO is new Ada.Text_IO.Modular_IO(Win32.UINT);
    package MMText_IO is new
Ada.Text_IO.Modular_IO(Win32.Mmsystem.MMRESULT);

    procedure Free is new
Ada.Unchecked_Deallocation(Win32.Mmsystem.MIDIINCAPS,
Win32.Mmsystem.LPMIDIINCAPS);
    procedure Free is new
Ada.Unchecked_Deallocation(Win32.Mmsystem.MIDIOUTCAPS,
Win32.Mmsystem.LPMIDIOUTCAPS);

begin
   NumInputDevices := Win32.Mmsystem.midiInGetNumDevs;
   NumOutputDevices := Win32.Mmsystem.midiOutGetNumDevs;
   midiInCaps  := new Win32.Mmsystem.MIDIINCAPS;
   midiOutCaps := new Win32.Mmsystem.MIDIOUTCAPS;

   Ada.Text_IO.Put("There are ");
   UINTText_IO.Put(NumInputDevices, 0);
   Ada.Text_IO.Put(" input devices available, and ");
   UINTText_IO.Put(NumOutputDevices, 0);
   Ada.Text_IO.Put_Line(" output devices available.");

   if NumInputDevices > 0
   then
      Ada.Text_IO.New_Line;
      Ada.Text_IO.Put("The ");
      UINTText_IO.Put(NumInputDevices, 0);
      Ada.Text_IO.Put_Line(" input devices are:");
      Ada.Text_IO.New_Line;

      for devId in Win32.UINT range 0..(NumInputDevices - 1)
      loop
         res := Win32.Mmsystem.midiInGetDevCaps(devId,
                                                midiInCaps,

(Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size));
         UINTText_IO.Put(devId, 0);
         Ada.Text_IO.Put(") ");
         if res = Win32.Mmsystem.MMSYSERR_NOERROR
         then
            Ada.Text_IO.Put("szPname = ");
            Ada.Text_IO.Put_Line(To_Ada(To_C(midiInCaps.szPname)));
         else
            Ada.Text_IO.Put("Query Failed. Returned ");
            MMText_IO.Put(res, 0);
         end if;
         Ada.Text_IO.New_Line;
      end loop;
   end if;

   if NumOutputDevices > 0
   then
      Ada.Text_IO.New_Line;
      Ada.Text_IO.Put("The ");
      UINTText_IO.Put(NumOutputDevices, 0);
      Ada.Text_IO.Put_Line(" output devices are:");
      Ada.Text_IO.New_Line;

      for devId in Win32.UINT range 0..(NumOutputDevices - 1)
      loop
         res := Win32.Mmsystem.midiOutGetDevCaps(devId,
                                                 midiOutCaps,

(Win32.Mmsystem.MIDIOUTCAPS'size * Win32.BYTE'size));
         UINTText_IO.Put(devId, 0);
         Ada.Text_IO.Put(") ");
         if res = Win32.Mmsystem.MMSYSERR_NOERROR
         then
            Ada.Text_IO.Put("szPname = ");
            Ada.Text_IO.Put_Line(To_Ada(To_C(midiOutCaps.szPname)));
         else
            Ada.Text_IO.Put("Query Failed. Returned ");
            MMText_IO.Put(res, 0);
         end if;
         Ada.Text_IO.New_Line;
      end loop;
   end if;

   Free(midiInCaps);
   Free(midiOutCaps);
   
end MidiDevs;

Re: This MIDI stuff, would someone be interested in reviewing my code?

[Gautier write-only]

Probably the most urgent thing is to make a package called MIDI, in
the body of which you hide all the Win32 / C things.
More precisely:
- hide the Win32 references
- use the C strings only in midi.adb
- define constants, when possible, enumerated types which match
Win32's
Otherwise your code will grow into something very, very messy (check
AdaGIDE's sources as the example of what to be avoided).
_________________________________________________________
Gautier's Ada programming -- http://sf.net/users/gdemont/
NB: For a direct answer, e-mail address on the following web site:
http://www.fechtenafz.ethz.ch/wm_email.htm

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Mon, 8 Mar 2010 03:52:04 -0800 (PST), Gautier write-only
<gautier_niouzes@hotmail.com> wrote:

>Probably the most urgent thing is to make a package called MIDI, in
>the body of which you hide all the Win32 / C things.
>More precisely:
>- hide the Win32 references
>- use the C strings only in midi.adb
>- define constants, when possible, enumerated types which match
>Win32's
>Otherwise your code will grow into something very, very messy (check
>AdaGIDE's sources as the example of what to be avoided).

Oh - of course. Those things would be the next step. This is just a
little noddy thing to get an idea of how to use it. For what it's
worth though, I've already got loads of examples of what should be
avoided :-}

Re: This MIDI stuff, would someone be interested in reviewing my code?

[Jeffrey R. Carter]

John McCabe wrote:
> 
> 1) Perhaps I should be using limited withs in some places to get
> access to the primitive operators/functions of the stuff in
> Interfaces.C/.strings and Win32 etc.

That's not what "limited with" is for.

I notice that you "use" a lot of pkgs, but then refer to things using full 
names. You should get rid of those "use" clauses, and add "use type" clauses in 
your declarative part as needed.

> 3) Would it be more appropriate to use something like
> Win32.UINT'Image() instead of getting an instantiation of the
> Modular_IO package?

That's a matter of taste and needed functionality.

My first comments would be to use common Ada naming conventions: Dev_ID, 
Num_Input_Devices, and so on.

I also prefer to see "loop" and "then" on the same line as "for" and "if". This 
looks like a C-family person thinking they're equivalent to '{'.

I would make Numinputdevices and Numoutputdevices constants. For this small 
example, I would do the same with Midiincaps and Midioutcaps; there's really no 
need to free them, since they're around for the entire program, and will be 
freed when the program exits.

> with Interfaces.C; use Interfaces.C;
> with Interfaces.C.Strings; use Interfaces.C.Strings;

"with Interfaces.C.Strings;" implies "with Interfaces; with Interfaces.C;", so 
there's no reason to have both.

> with Win32; use Win32;
> with Win32.Mmsystem; use Win32.Mmsystem;

Ditto.

>       for devId in Win32.UINT range 0..(NumInputDevices - 1)
>       loop

These parentheses are unnecessary.

>          res := Win32.Mmsystem.midiInGetDevCaps(devId,
>                                                 midiInCaps,
> 
> (Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size));

So are the internal ones here.

You've already been warned about sprinkling OS-dependent stuff throughout your code.

-- 
Jeff Carter
"C's solution to this [variable-sized array parameters] has real
problems, and people who are complaining about safety definitely
have a point."
Dennis Ritchie
25

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Mon, 08 Mar 2010 10:24:26 -0700, "Jeffrey R. Carter"
<spam.jrcarter.not@spam.acm.org> wrote:

>John McCabe wrote:
>> 
>> 1) Perhaps I should be using limited withs in some places to get
>> access to the primitive operators/functions of the stuff in
>> Interfaces.C/.strings and Win32 etc.
>
>That's not what "limited with" is for.

>I notice that you "use" a lot of pkgs, but then refer to things using full 
>names. You should get rid of those "use" clauses, and add "use type" clauses in 
>your declarative part as needed.

Ah - thanks. I was getting confused there between limited with and use
type. You've clarified that for me.

>> 3) Would it be more appropriate to use something like
>> Win32.UINT'Image() instead of getting an instantiation of the
>> Modular_IO package?

>That's a matter of taste and needed functionality.

>My first comments would be to use common Ada naming conventions: Dev_ID, 
>Num_Input_Devices, and so on.

Ok - I'll see what I can do.

>I also prefer to see "loop" and "then" on the same line as "for" and "if". This 
>looks like a C-family person thinking they're equivalent to '{'.

I was thinking of them being more equivalent to Ada's begin for loops.
I like the idea of having a visual clue, be it a begin, loop, then, {
or whatever on a line on its own to show clearly that the following
code is indented because it's within that construct.

>I would make Numinputdevices and Numoutputdevices constants.

They're not constants though - they're discovered from the
midiInGetNumDevs and midiOutGetnumDevs functions.

>For this small 
>example, I would do the same with Midiincaps and Midioutcaps; there's really no 
>need to free them, since they're around for the entire program, and will be 
>freed when the program exits.

The point of allocating/freeing them was to get round the fact that,
when they were declared in the declarative part of the main function,
I seemed to have to use 'Unchecked_Access when passing them to
midiInGetDevCaps/midiOutGetDevCaps.

>> with Interfaces.C; use Interfaces.C;
>> with Interfaces.C.Strings; use Interfaces.C.Strings;
>
>"with Interfaces.C.Strings;" implies "with Interfaces; with Interfaces.C;", so 
>there's no reason to have both.

Ok - thanks.

>> with Win32; use Win32;
>> with Win32.Mmsystem; use Win32.Mmsystem;

>Ditto.

Ditto too :-)

>>       for devId in Win32.UINT range 0..(NumInputDevices - 1)
>>       loop
>
>These parentheses are unnecessary.

Yes, I agree. I like to make things obvious though :-)

>>          res := Win32.Mmsystem.midiInGetDevCaps(devId,
>>                                                 midiInCaps,
>> 
>> (Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size));
>
>So are the internal ones here.

True.

>You've already been warned about sprinkling OS-dependent stuff throughout your code.

Yip.

Thanks for those comments. Much appreciated.

Re: This MIDI stuff, would someone be interested in reviewing my code?

[Brian Drummond]

On Tue, 09 Mar 2010 10:21:00 +0000, John McCabe <john@nospam.assen.demon.co.uk>
wrote:

>On Mon, 08 Mar 2010 10:24:26 -0700, "Jeffrey R. Carter"
><spam.jrcarter.not@spam.acm.org> wrote:

>>I would make Numinputdevices and Numoutputdevices constants.
>
>They're not constants though - they're discovered from the
>midiInGetNumDevs and midiOutGetnumDevs functions.

But you can call those functions in an initialisation clause on the declaration.
Then they remain constant within the begin..end section, so you can declare them
as constants.

>>For this small 
>>example, I would do the same with Midiincaps and Midioutcaps;

>The point of allocating/freeing them was to get round the fact that,
>when they were declared in the declarative part of the main function,
>I seemed to have to use 'Unchecked_Access when passing them to
>midiInGetDevCaps/midiOutGetDevCaps.

Look into the "aliased" keyword. Unlike C, the compiler will assume a local
variable is never aliased (and can be optimised) - unless you declare it
aliased, to say there may be a pointer to it. Declare them aliased and you
shouldn't need Unchecked_Access.

- Brian

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Tue, 09 Mar 2010 10:21:00 +0000, John McCabe
<john@nospam.assen.demon.co.uk> wrote:

>>I would make Numinputdevices and Numoutputdevices constants.

>They're not constants though - they're discovered from the
>midiInGetNumDevs and midiOutGetnumDevs functions.

Just realised what you mean; they're constant through the life of the
program so do

procedure MidiDevs is
   blah

   Num_Input_Devices : Win32.UINT := Win32.Mmsystem.midiInGetNumDevs;
   <same for output>
begin

   blah

end MidiDevs;

Yes, that would be possible.

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Tue, 09 Mar 2010 12:10:16 +0000, Brian Drummond
<brian_drummond@btconnect.com> wrote:

>On Tue, 09 Mar 2010 10:21:00 +0000, John McCabe <john@nospam.assen.demon.co.uk>
>wrote:
>
>>On Mon, 08 Mar 2010 10:24:26 -0700, "Jeffrey R. Carter"
>><spam.jrcarter.not@spam.acm.org> wrote:
>
>>>I would make Numinputdevices and Numoutputdevices constants.
>>
>>They're not constants though - they're discovered from the
>>midiInGetNumDevs and midiOutGetnumDevs functions.
>
>But you can call those functions in an initialisation clause on the declaration.
>Then they remain constant within the begin..end section, so you can declare them
>as constants.

Yes - you're right - I'd just realised that before reading your
message.

>>>For this small 
>>>example, I would do the same with Midiincaps and Midioutcaps;
>
>>The point of allocating/freeing them was to get round the fact that,
>>when they were declared in the declarative part of the main function,
>>I seemed to have to use 'Unchecked_Access when passing them to
>>midiInGetDevCaps/midiOutGetDevCaps.
>
>Look into the "aliased" keyword. Unlike C, the compiler will assume a local
>variable is never aliased (and can be optimised) - unless you declare it
>aliased, to say there may be a pointer to it. Declare them aliased and you
>shouldn't need Unchecked_Access.

I had them declared as aliased. With just 'access on it the compiler
said "blah blah blah can't do it because it's not global" (or
something like that). I'll check again, but that was the gist of it.

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Tue, 09 Mar 2010 12:26:21 +0000, John McCabe
<john@nospam.assen.demon.co.uk> wrote:

>On Tue, 09 Mar 2010 12:10:16 +0000, Brian Drummond
><brian_drummond@btconnect.com> wrote:

>>>>For this small 
>>>>example, I would do the same with Midiincaps and Midioutcaps;

>>>The point of allocating/freeing them was to get round the fact that,
>>>when they were declared in the declarative part of the main function,
>>>I seemed to have to use 'Unchecked_Access when passing them to
>>>midiInGetDevCaps/midiOutGetDevCaps.

>>Look into the "aliased" keyword. Unlike C, the compiler will assume a local
>>variable is never aliased (and can be optimised) - unless you declare it
>>aliased, to say there may be a pointer to it. Declare them aliased and you
>>shouldn't need Unchecked_Access.

>I had them declared as aliased. With just 'access on it the compiler
>said "blah blah blah can't do it because it's not global" (or
>something like that). I'll check again, but that was the gist of it.

mididevs.adb:56:49: non-local pointer cannot point to local object

Note - this is an updated version so the lines aren't the same number!

That's with something like:

procedure MidiDevs is
   Midi_In_Caps : aliased Win32.Mmsystem.MIDIINCAPS;
   <blah>
begin
   <blah>
   res := Win32.Mmsystem.midiInGetDevCaps(Device_ID,
                                          Midi_In_Caps'Access,

Win32.Mmsystem.MIDIOUTCAPS'size 
                                             * Win32.BYTE'size);
   <blah>
end MidiDevs;

FWIW - I keep mentioning it's a long time since I used Ada, so there
may be something in there I've forgotten - I'll check my books!

Re: This MIDI stuff, would someone be interested in reviewing my code?

[Martin]

On Mar 9, 12:24 pm, John McCabe <j...@nospam.assen.demon.co.uk> wrote:
> On Tue, 09 Mar 2010 10:21:00 +0000, John McCabe
>
> <j...@nospam.assen.demon.co.uk> wrote:
> >>I would make Numinputdevices and Numoutputdevices constants.
> >They're not constants though - they're discovered from the
> >midiInGetNumDevs and midiOutGetnumDevs functions.
>
> Just realised what you mean; they're constant through the life of the
> program so do
>
> procedure MidiDevs is
>    blah
>
>    Num_Input_Devices : Win32.UINT := Win32.Mmsystem.midiInGetNumDevs;
>    <same for output>
> begin
>
>    blah
>
> end MidiDevs;
>
> Yes, that would be possible.

Better yet:
   Num_Input_Devices : constant Win32.UINT :=
Win32.Mmsystem.midiInGetNumDevs;

Cheers
-- Martin

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Tue, 9 Mar 2010 05:11:25 -0800 (PST), Martin
<martin.dowie@btopenworld.com> wrote:

>On Mar 9, 12:24�pm, John McCabe <j...@nospam.assen.demon.co.uk> wrote:
>> On Tue, 09 Mar 2010 10:21:00 +0000, John McCabe
>>
>> <j...@nospam.assen.demon.co.uk> wrote:
>> >>I would make Numinputdevices and Numoutputdevices constants.
>> >They're not constants though - they're discovered from the
>> >midiInGetNumDevs and midiOutGetnumDevs functions.
>>
>> Just realised what you mean; they're constant through the life of the
>> program so do
>>
>> procedure MidiDevs is
>> � �blah
>>
>> � �Num_Input_Devices : Win32.UINT := Win32.Mmsystem.midiInGetNumDevs;
>> � �<same for output>
>> begin
>>
>> � �blah
>>
>> end MidiDevs;
>>
>> Yes, that would be possible.
>
>Better yet:
>   Num_Input_Devices : constant Win32.UINT :=
>Win32.Mmsystem.midiInGetNumDevs;

Aaarrrggh - of course! The things that happen when you rush to type
something :-)

Re: This MIDI stuff, would someone be interested in reviewing my code?

[Stephen Leake]

John McCabe <john@nospam.assen.demon.co.uk> writes:

> On Tue, 09 Mar 2010 12:26:21 +0000, John McCabe
> <john@nospam.assen.demon.co.uk> wrote:
>
>>On Tue, 09 Mar 2010 12:10:16 +0000, Brian Drummond
>><brian_drummond@btconnect.com> wrote:
>
>>>>>For this small 
>>>>>example, I would do the same with Midiincaps and Midioutcaps;
>
>>>>The point of allocating/freeing them was to get round the fact that,
>>>>when they were declared in the declarative part of the main function,
>>>>I seemed to have to use 'Unchecked_Access when passing them to
>>>>midiInGetDevCaps/midiOutGetDevCaps.
>
>>>Look into the "aliased" keyword. Unlike C, the compiler will assume a local
>>>variable is never aliased (and can be optimised) - unless you declare it
>>>aliased, to say there may be a pointer to it. Declare them aliased and you
>>>shouldn't need Unchecked_Access.
>
>>I had them declared as aliased. With just 'access on it the compiler
>>said "blah blah blah can't do it because it's not global" (or
>>something like that). I'll check again, but that was the gist of it.
>
> mididevs.adb:56:49: non-local pointer cannot point to local object
>
> Note - this is an updated version so the lines aren't the same number!
>
> That's with something like:
>
> procedure MidiDevs is
>    Midi_In_Caps : aliased Win32.Mmsystem.MIDIINCAPS;

This object is declared in a procedure, and is therefore allocated on
the stack.

> begin
>    <blah>
>    res := Win32.Mmsystem.midiInGetDevCaps(Device_ID,
>                                           Midi_In_Caps'Access,

This function takes a parameter of a global access type.

So in theory, the function could store a copy of the pointer, and then
use it after MidiDevs has returned, and the actual object disappeared. 

That's why this is illegal.

However, if MidiDevs is really your main procedure, then Midi_In_Caps
will never disappear, and 'Unchecked_Access is safe.

-- 
-- Stephe

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

On Wed, 10 Mar 2010 06:12:11 -0500, Stephen Leake
<stephen_leake@stephe-leake.org> wrote:

>John McCabe <john@nospam.assen.demon.co.uk> writes:
>
>>>I had them declared as aliased. With just 'access on it the compiler
>>>said "blah blah blah can't do it because it's not global" (or
>>>something like that). I'll check again, but that was the gist of it.
>>
>> mididevs.adb:56:49: non-local pointer cannot point to local object
>>
>> Note - this is an updated version so the lines aren't the same number!
>>
>> That's with something like:
>>
>> procedure MidiDevs is
>>    Midi_In_Caps : aliased Win32.Mmsystem.MIDIINCAPS;
>
>This object is declared in a procedure, and is therefore allocated on
>the stack.

That's right.

>> begin
>>    <blah>
>>    res := Win32.Mmsystem.midiInGetDevCaps(Device_ID,
>>                                           Midi_In_Caps'Access,
>
>This function takes a parameter of a global access type.
>
>So in theory, the function could store a copy of the pointer, and then
>use it after MidiDevs has returned, and the actual object disappeared. 
>
>That's why this is illegal.

Exactly.

>However, if MidiDevs is really your main procedure, then Midi_In_Caps
>will never disappear, and 'Unchecked_Access is safe.

Fair point. Thanks.

Re: This MIDI stuff, would someone be interested in reviewing my code?

[Christophe Chaumet]

John McCabe a �crit :
> Hi
>
> It's still early days but, if I'm going to be using Ada to try to
> build this app I want, it would be nice to write it in a style that
> looks appropriate. I'm aware of the Q&S guide but I was hoping that
> someone could take a quick look at the code I've written (it's only 80
> lines or so, and it's down below) and see if there's anything
> obviously stupid I'm doing.
>
> My specific thoughts on this are:
>
> 1) Perhaps I should be using limited withs in some places to get
> access to the primitive operators/functions of the stuff in
> Interfaces.C/.strings and Win32 etc.
>
> 2) The for loops: for devId in Win32.UINT range 0..(NumOutputDevices -
> 1) etc. These are protected by a "if NumOutputDevices < 0" condition
> but before I realised my mistake I found that when NumOutputDevices is
> 0, the loop executes as many times as it can before it crashed. This
> was obviously because NumOutputDevices was 0, so the range
> "0..(NumOutputDevices - 1)" was 0..4929blahblah due to Win32.UINT
> being a modular type. I looked at the option to use something like:
>    for index in Win32.UINT range 1..NumOuputDevices loop
>       declare
>          devId : Win32.UINT := index - 1;
>       begin
>          ...
>       end;
>    end loop;
> but stuck with the original with the conditional round it.
>
> 3) Would it be more appropriate to use something like
> Win32.UINT'Image() instead of getting an instantiation of the
> Modular_IO package?
>
> Anyway - thanks to anyone who can be bothered to look at this. It will
> be much appreciated, and thanks for everyone's help so far.
>
> John
>
>
> ===================
> with Ada.Text_IO;
> with Ada.Unchecked_Deallocation;
>
> with Interfaces.C; use Interfaces.C;
> with Interfaces.C.Strings; use Interfaces.C.Strings;
>
> with Win32; use Win32;
> with Win32.Mmsystem; use Win32.Mmsystem;
>
> procedure MidiDevs is
>     NumInputDevices  : Win32.UINT;
>     NumOutputDevices : Win32.UINT;
>
>     res         : Win32.Mmsystem.MMRESULT;
>     midiInCaps  : Win32.Mmsystem.LPMIDIINCAPS;
>     midiOutCaps : Win32.Mmsystem.LPMIDIOUTCAPS;
>
>     package UINTText_IO is new Ada.Text_IO.Modular_IO(Win32.UINT);
>     package MMText_IO is new
> Ada.Text_IO.Modular_IO(Win32.Mmsystem.MMRESULT);
>
>     procedure Free is new
> Ada.Unchecked_Deallocation(Win32.Mmsystem.MIDIINCAPS,
> Win32.Mmsystem.LPMIDIINCAPS);
>     procedure Free is new
> Ada.Unchecked_Deallocation(Win32.Mmsystem.MIDIOUTCAPS,
> Win32.Mmsystem.LPMIDIOUTCAPS);
>
> begin
>    NumInputDevices := Win32.Mmsystem.midiInGetNumDevs;
>    NumOutputDevices := Win32.Mmsystem.midiOutGetNumDevs;
>    midiInCaps  := new Win32.Mmsystem.MIDIINCAPS;
>    midiOutCaps := new Win32.Mmsystem.MIDIOUTCAPS;
>
>    Ada.Text_IO.Put("There are ");
>    UINTText_IO.Put(NumInputDevices, 0);
>    Ada.Text_IO.Put(" input devices available, and ");
>    UINTText_IO.Put(NumOutputDevices, 0);
>    Ada.Text_IO.Put_Line(" output devices available.");
>
>    if NumInputDevices > 0
>    then
>       Ada.Text_IO.New_Line;
>       Ada.Text_IO.Put("The ");
>       UINTText_IO.Put(NumInputDevices, 0);
>       Ada.Text_IO.Put_Line(" input devices are:");
>       Ada.Text_IO.New_Line;
>
>       for devId in Win32.UINT range 0..(NumInputDevices - 1)
>       loop
>          res := Win32.Mmsystem.midiInGetDevCaps(devId,
>                                                 midiInCaps,
>
> (Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size));
>          UINTText_IO.Put(devId, 0);
>          Ada.Text_IO.Put(") ");
>          if res = Win32.Mmsystem.MMSYSERR_NOERROR
>          then
>             Ada.Text_IO.Put("szPname = ");
>             Ada.Text_IO.Put_Line(To_Ada(To_C(midiInCaps.szPname)));
>          else
>             Ada.Text_IO.Put("Query Failed. Returned ");
>             MMText_IO.Put(res, 0);
>          end if;
>          Ada.Text_IO.New_Line;
>       end loop;
>    end if;
>
>    if NumOutputDevices > 0
>    then
>       Ada.Text_IO.New_Line;
>       Ada.Text_IO.Put("The ");
>       UINTText_IO.Put(NumOutputDevices, 0);
>       Ada.Text_IO.Put_Line(" output devices are:");
>       Ada.Text_IO.New_Line;
>
>       for devId in Win32.UINT range 0..(NumOutputDevices - 1)
>       loop
>          res := Win32.Mmsystem.midiOutGetDevCaps(devId,
>                                                  midiOutCaps,
>
> (Win32.Mmsystem.MIDIOUTCAPS'size * Win32.BYTE'size));
>          UINTText_IO.Put(devId, 0);
>          Ada.Text_IO.Put(") ");
>          if res = Win32.Mmsystem.MMSYSERR_NOERROR
>          then
>             Ada.Text_IO.Put("szPname = ");
>             Ada.Text_IO.Put_Line(To_Ada(To_C(midiOutCaps.szPname)));
>          else
>             Ada.Text_IO.Put("Query Failed. Returned ");
>             MMText_IO.Put(res, 0);
>          end if;
>          Ada.Text_IO.New_Line;
>       end loop;
>    end if;
>
>    Free(midiInCaps);
>    Free(midiOutCaps);
>    
> end MidiDevs;
>
>   
Here is a working code: http://sourceforge.net/projects/canta/ written 
in Ada.

Re: This MIDI stuff, would someone be interested in reviewing my code?

[John McCabe]

Christophe,

>John McCabe a �crit :
>> Hi
>>
>> It's still early days but, if I'm going to be using Ada to try to
>> build this app I want, it would be nice to write it in a style that
>> looks appropriate. I'm aware of the Q&S guide but I was hoping that
>> someone could take a quick look at the code I've written (it's only 80
>> lines or so, and it's down below) and see if there's anything
>> obviously stupid I'm doing.

>Here is a working code: http://sourceforge.net/projects/canta/ written 
>in Ada.

Thank you for pointing me to that code. I really appreciate it but
unfortunately there only seems to be a small amount of it that fits in
with the application I'm thinking of developing, and that's the
opening of the MIDI output port and sending short messages.

That will be useful, but my main challenges will be handling the MIDI
input and the GUI. From the GUI point of view, I see that you're
directly using the Windows functions, like CreateWindow and so on. I'm
hoping to be able to use Qt to help this become cross-platform (I know
my calls to midiInGetNumDevs etc are all explicit above, but I hope to
abstract an interface to the midi functionality once I've proven that
I can do the basic stuff on windows).

The other thing is that all my MIDI dealings will be with System
Exclusive messages whereas yours appears to only use short messages to
the MIDI output device.

I certainly appreciate you taking the time to reply though, so thank
you very much.

John