Some help for a C++ guy

Some help for a C++ guy

[defaultuserbr]

I've inherited some code that I'm expanding, and my Ada skills are
somewhat weak, I mostly do C and C++.

Our system will have multiple messages defined, with three essential
components, a data buffer, a status buffer, and a callback. Currently
this is defined in a generic package. The package spec follows:

with MIL_STD_1553;
with OS_Types;
generic
package Message_Handler is

   package M1553 renames MIL_STD_1553;

   procedure Message_Callback (Status : in M1553.IO_Status_T);

   function Num_Message_Callbacks return OS_Types.Unsigned_Word;

   function Num_Errors return OS_Types.Unsigned_Word;

   Data_Buf_Ptr : constant M1553.Data_Buf_VPtr_T := new M1553.Data_Buf;
   Stat_Buf_Ptr : constant M1553.Status_Buf_VPtr_T := new
M1553.Status_Buf_T;


end Message_Handler;


Then in the implementation code, there's stuff like:

   package Message_0_Handler is new Message_Handler;
   package Message_5_Handler is new Message_Handler;

The buffers and the callback have to be registered for each of these
instantiated messages through another subroutine.

I don't much like this. It's cumbersome even for a couple of messages,
let alone when we grow to 20 or more, because we have duplicated code
for each registration.

What I'd like is to have some sort of array of the messages. That way
one could iterate over the array and reduce duplicate code and chance
for errors. I guess I'm thinking an OO approach, as long as the various
members could be passed as arguments to the registration routines.

We are working in Ada95. However one twist is that we are using Gnat
Pro with  something called a certifiable subset. Here some stuff about
it:

http://www.extensionmedia.com/windriver/datasheet.php?ds=2

I don't know much more myself, other than we can't use tasking. I
understand that such limitations may make this too
implementation-specific for a general newsgroup. Any help would be
appreciated.



Brian

Re: Some help for a C++ guy

[Ed Falis]

On Thu, 07 Apr 2005 18:14:22 -0400, <defaultuserbr@yahoo.com> wrote:

> I don't know much more myself, other than we can't use tasking. I
> understand that such limitations may make this too
> implementation-specific for a general newsgroup. Any help would be
> appreciated.
>
>
>
> Brian
>

Brian,

Dumb question: is there some reason you're not asking AdaCore?

- Ed

Re: Some help for a C++ guy

[defaultuserbr]

Ed Falis wrote:
> On Thu, 07 Apr 2005 18:14:22 -0400, <defaultuserbr@yahoo.com> wrote:
>
> > I don't know much more myself, other than we can't use tasking. I
> > understand that such limitations may make this too
> > implementation-specific for a general newsgroup. Any help would be
> > appreciated.

> Dumb question: is there some reason you're not asking AdaCore?


Well, my question really isn't a problem with the tool per se, I'm just
trying to come up with a design that's better. I doubt they would
answer such questions.

I only mentioned the restriction to forestall anyone suggestioning a
tasking approach, as I had already tried that on the advice of a guy
working for the company in Philadelphia.



Brian

Re: Some help for a C++ guy

[Ed Falis]

On Thu, 07 Apr 2005 19:18:52 -0400, <defaultuserbr@yahoo.com> wrote:

> Well, my question really isn't a problem with the tool per se, I'm just
> trying to come up with a design that's better. I doubt they would
> answer such questions.

Send me a private email - AdaCore definitely does that kind of support as  
part of the normal package.

- Ed

Re: Some help for a C++ guy

[defaultuserbr]

Ed Falis wrote:
> On Thu, 07 Apr 2005 19:18:52 -0400, <defaultuserbr@yahoo.com> wrote:
>
> > Well, my question really isn't a problem with the tool per se, I'm
just
> > trying to come up with a design that's better. I doubt they would
> > answer such questions.
>
> Send me a private email - AdaCore definitely does that kind of
support as
> part of the normal package.


Ok, I will do that shortly.



Brian

Re: Some help for a C++ guy

[Simon Wright]

defaultuserbr@yahoo.com writes:

> Ed Falis wrote:
> > On Thu, 07 Apr 2005 18:14:22 -0400, <defaultuserbr@yahoo.com> wrote:
> >
> > > I don't know much more myself, other than we can't use
> > > tasking. I understand that such limitations may make this too
> > > implementation-specific for a general newsgroup. Any help would
> > > be appreciated.
> 
> > Dumb question: is there some reason you're not asking AdaCore?
> 
> 
> Well, my question really isn't a problem with the tool per se, I'm
> just trying to come up with a design that's better. I doubt they
> would answer such questions.

You should certainly try them and see! It is the sort of question I
would be quite happy asking on our support contract.

> I only mentioned the restriction to forestall anyone suggestioning a
> tasking approach, as I had already tried that on the advice of a guy
> working for the company in Philadelphia.

It sounds from your original question as though you want to use tagged
types, roughly the equivalent of classes & inheritance in C++.

Looking at AdaCore's site
<http://www.adacore.com/gnatpro_high_integrity.php> it says

   GNAT Pro HIE predefines three customized run-time libraries that
   are particularly useful for safety-critical systems.

   These correspond to Ada subsets referred to as the High Integrity
   Profiles:

       * the Zero Footprint (or ZFP) profile, a minimal subset
         requiring no run-time support

       * the Ravenscar profile, a superset of ZFP that supplies the
         Ravenscar tasking features

       * the Cert profile, a superset of ZFP that is based on
         requirements from several major aerospace companies,
         comprising exception handling and some additional features

   The exact set of High-Integrity profiles provided by a GNAT Pro HIE
   implementation depends on the platform.

so I think you at least need to find out what the Cert profile for
your target actually is -- hard to work out how to approach the job
without knowing that!

-- 
Simon Wright                               100% Ada, no bugs.

Re: Some help for a C++ guy

[defaultuserbr]

Simon Wright wrote:

> so I think you at least need to find out what the Cert profile for
> your target actually is -- hard to work out how to approach the job
> without knowing that!


That's probably the case. This is some code I inherited from another
guy who left the project, and all I'm really doing is finishing a
sample client application as a test driver and proof of concept. Then
it all gets turned over to the people in other part of the country who
are developing the real client applications. Hence I can't spend *too*
much time on doing something different.

It's just my nature as a software engineer to abhor inelegant
approaches.


Brian

Re: Some help for a C++ guy

[defaultuserbr]

Simon Wright wrote:


> It sounds from your original question as though you want to use
tagged
> types, roughly the equivalent of classes & inheritance in C++.

Here's my current attempt:

with MIL_STD_1553;
with OS_Types;

package Message_Handler is

   package M1553 renames MIL_STD_1553;

   type Handler is
      tagged record
         Data_Buf_Ptr   :  M1553.Data_Buf_VPtr_T := new M1553.Data_Buf;
         Stat_Buf_Ptr   :  M1553.Status_Buf_VPtr_T :=
           new M1553.Status_Buf_T;
      end record;


      -- the following are the primitive operations of the type
      procedure Message_Callback (Status : in M1553.IO_Status_T);

      function Num_Message_Callbacks return OS_Types.Unsigned_Word;

      function Num_Errors return OS_Types.Unsigned_Word;


end Message_Handler;

That compiles OK. In the client test package, I have:

with Message_Handler;

pragma Elaborate_All(Message_Handler);

   Handlers : Array (0..1) of Message_Handler.Handler;

That's all good too.

However, the first place that uses something from Message_Handler:

      Callbacks := Handlers(0).Num_Message_Callbacks;

Fails to compile, with an error message:

no selector "Num_Message_Callbacks" for type Handler defined at
message_handler.ads


So obviously I'm either defining things wrong or using what I have
defined incorrectly.



Brian

Re: Some help for a C++ guy

[tmoran]

>  Handlers : Array (0..1) of Message_Handler.Handler;
> ...
>     Callbacks := Handlers(0).Num_Message_Callbacks;
  In Ada 95 you would say
      Callbacks := Num_Message_Callbacks(Handler(0));
ie, you pass the (tagged) parameter as a parameter, not as a prefix.
(The prefix form will apparently be allowed by the upcoming revision.)
  With the generic version you clearly did not have inheritance, so do
want to add it now?  You must be planning some significant architectural
changes.  Why is a Handler a tagged record rather than an untagged one?

Re: Some help for a C++ guy

[defaultuserbr]

tmo...@acm.org wrote:
> >  Handlers : Array (0..1) of Message_Handler.Handler;
> > ...
> >     Callbacks := Handlers(0).Num_Message_Callbacks;
>   In Ada 95 you would say
>       Callbacks := Num_Message_Callbacks(Handler(0));
> ie, you pass the (tagged) parameter as a parameter, not as a prefix.

Which I gather requires a change in the function's signature. That's
not a big deal for that one, but the callback is another matter.

> (The prefix form will apparently be allowed by the upcoming
revision.)
>   With the generic version you clearly did not have inheritance, so
do
> want to add it now?  You must be planning some significant
architectural
> changes.  Why is a Handler a tagged record rather than an untagged
one?

Yes, that's true. I was thinking about inheritance because I may want
different message handlers for different message types. But it's not
essential. I'm not too optimistic about this even working, due to the
problem (I think) with the callback signature.



Brian

Re: Some help for a C++ guy

[defaultuserbr]

defaultuserbr@yahoo.com wrote:
>
>    package M1553 renames MIL_STD_1553;
>
>    type Handler is
>       tagged record
>          Data_Buf_Ptr   :  M1553.Data_Buf_VPtr_T := new
M1553.Data_Buf;
>          Stat_Buf_Ptr   :  M1553.Status_Buf_VPtr_T :=
>            new M1553.Status_Buf_T;
>       end record;
>
>       procedure Message_Callback (Status : in M1553.IO_Status_T);
>
>       function Num_Message_Callbacks return OS_Types.Unsigned_Word;
>
>       function Num_Errors return OS_Types.Unsigned_Word;
>
>
> end Message_Handler;
>
> That compiles OK. In the client test package, I have:
>
> with Message_Handler;
>
> pragma Elaborate_All(Message_Handler);
>
>    Handlers : Array (0..1) of Message_Handler.Handler;
>
> That's all good too.
>
> However, the first place that uses something from Message_Handler:
>
>       Callbacks := Handlers(0).Num_Message_Callbacks;
>
> Fails to compile, with an error message:
>
> no selector "Num_Message_Callbacks" for type Handler defined at
> message_handler.ads
>
>
> So obviously I'm either defining things wrong or using what I have
> defined incorrectly.

>From research, looks like these subroutines that would be class methods
or whatever you want to call them must, in the words of on online
source:

"Non-virtual, non-const, non-static member functions map onto
subprograms, within the same package as the tagged type, whos first
parameter is of that tagged type or an access to the tagged type, or
who returns such a type."


That's a problem because that callback subroutine needs to be passed to
a function expecting some of the type BC_Completion_Handler_T, defined
as:

   type BC_Completion_Handler_T is access
      procedure (Status : in IO_Status_T);


Changing the that type would then break a bunch of other code.

I'm not sure I'm going to be able to find a solution, I'm afraid.


Brian

Re: Some help for a C++ guy

[tmoran]

>That's a problem because that callback subroutine needs to be passed to
>a function expecting some of the type BC_Completion_Handler_T, defined
>as:
>
>   type BC_Completion_Handler_T is access
>      procedure (Status : in IO_Status_T);
>
>Changing the that type would then break a bunch of other code.

So you need to specify for a callback the particular Message_* that
it pertains to?  Just make a set of wrapper routines:
    procedure Message_0_Completion_Handler(Status : in IO_Status_T) is
    begin
      Completion_Handler(Message(0), Status);
    end Message_0_Completion_Handler;
etc.

and register the specific Message_*_Completion_Handler procedures.
I presume this registration is done once so even if you add a new
Message_237_Completion_Handler it's a minor addition to a single package.

I'm presuming you want to make small changes to the existing code.
If you want to use tagged types you are contemplating larger architectural
changes.  In your current code, "registering a callback" means essentially
"note a particular routine, and a particular Message_n's data for future
use".  Passing an explicit tagged Handler parameter accomplishes the
"note a particular Message_n's data" part.  If the particular routine
is not in fact dynamic, but always BC_Completion_Handler, then instead
of registering the procedure callback you want to register the Message_n
data and always call BC_Completion_Handler(Registered_Message, Status);

Re: Some help for a C++ guy

[defaultuserbr]

tmo...@acm.org wrote:
> >That's a problem because that callback subroutine needs to be passed
to
> >a function expecting some of the type BC_Completion_Handler_T,
defined
> >as:
> >
> >   type BC_Completion_Handler_T is access
> >      procedure (Status : in IO_Status_T);
> >
> >Changing the that type would then break a bunch of other code.
>
> So you need to specify for a callback the particular Message_* that
> it pertains to?

Correct. Right now the very simple callback just increments a counter.
That way I can see how times a particlar message ran.

> Just make a set of wrapper routines:
>     procedure Message_0_Completion_Handler(Status : in IO_Status_T)
is
>     begin
>       Completion_Handler(Message(0), Status);
>     end Message_0_Completion_Handler;
> etc.
>
> and register the specific Message_*_Completion_Handler procedures.
> I presume this registration is done once so even if you add a new
> Message_237_Completion_Handler it's a minor addition to a single
package.

Plus an additional blurp of code to register that callback. What I
wanted to be able to do was have all the registration stuff in a loop.

So rather than a whole bunch of

       Stat := M1553.Register_Message_Callback(msg_id,
                Message_0_Handler.Message_Callback'access);

       if Stat /= M1553.No_Errors
          then
            GNAT.IO.Put_Line("Error Register_Message_Callback");
          end if;

Blocks, one for each message (and similar ones for the two buffers that
have to be registered), I'd have:

   for i in Handlers'range loop
       Stat := M1553.Register_Message_Callback(msg_id,
                Handlers(i).Message_Callback'access);

       if Stat /= M1553.No_Errors
          then
            GNAT.IO.Put_Line("Error Register_Message_Callback");
          end if;
   end loop;

I'd have been perfectly happy with a way to create an array of those
generic package instantiations.

> I'm presuming you want to make small changes to the existing code.
> If you want to use tagged types you are contemplating larger
architectural
> changes.

I don't want to break existing code.

> In your current code, "registering a callback" means essentially
> "note a particular routine, and a particular Message_n's data for
future
> use".

It means pass these to already written registration routines through an
established API.

> Passing an explicit tagged Handler parameter accomplishes the
> "note a particular Message_n's data" part.  If the particular routine
> is not in fact dynamic, but always BC_Completion_Handler, then
instead
> of registering the procedure callback you want to register the
Message_n
> data and always call BC_Completion_Handler(Registered_Message,
Status);

Well, there will be a specific callback function dedicated the each
message. For right now, as I said above, it just tracks how many times
the message ran. Later, they'd be expanded to do more things.

So each message needs to have a distinct data buffer, status buffer,
and callback subroutine.

This is tricky in C++ as well. Usually you go for callback objects or
functors rather than trying to pass a class member function.


Brian

Re: Some help for a C++ guy

[tmoran]

Why not save pointers to the Handler objects instead of the procedures?

You have something like:

1) type BC_Completion_Handler_T is access
     procedure (Status : in IO_Status_T);
   ...
2) -- in the routine that does callbacks you have:
   Saved_Completion_Handler_Ptr : BC_Completion_Handler_T;
   procedure Register_Message_Callback (msg_id : ... ;
                           Callback_Ptr : BC_Completion_Handler_T) is
   begin
     Saved_Completion_Handler_Ptr := Callback_Ptr;
   end Register_Message_Callback;

3) -- and then in various places therein you have the actual callbacks
   Saved_Completion_Handler_Ptr.all (Status);


change the above to something like:

1) type BC_Completion_Handler_T is access all Handler;
   ...
2) -- then in the routine that does callbacks you have:
   Saved_Completion_Handler_Ptr : BC_Completion_Handler_T;
   procedure Register_Message_Callback (msg_id : ... ;
                           Callback_Ptr : BC_Completion_Handler_T) is
   begin
     Saved_Completion_Handler_Ptr := Callback_Ptr;
   end Register_Message_Callback;

3) -- and then in various places therein you have the actual callbacks
   Completion_Handler (Saved_Completion_Handler_Ptr, Status);


that would require a single point change to (1), and a global replace
   Saved_Completion_Handler_Ptr.all (Status);
   =>
   Completion_Handler (Saved_Message_Ptr, Status);
Registration of an array of Handler could then be done in a loop.

Of course you would also do global replacements
BC_Completion_Handler_T   =>  BC_Message_T
Saved_Completion_Handler  =>  Saved_Message
Callback                  =>  To_Callback
so as not to confuse readers, leading to:

1) type BC_Message_T is access all Handler;
   ...
2) -- then in the routine that does callbacks you have:
   Saved_Message_Ptr : BC_Message_T;
   procedure Register_Message_To_Callback (msg_id : ... ;
                           To_Callback_Ptr : BC_Message_T) is
   begin
     Saved_Message_Ptr := To_Callback_Ptr;
   end Register_Message_To_Callback;

3) -- and then in various places therein you have the actual callbacks
   Completion_Handler (Saved_Message_Ptr, Status);

Re: Some help for a C++ guy

[defaultuserbr]

tmoran@acm.org wrote:
> Why not save pointers to the Handler objects instead of the
procedures?

That would probably be what I would do if I were designing it. However,
that code is all written, changing the API at this point could be a
problem.


[big snip of revised code]

> that would require a single point change to (1), and a global replace

Ok, I'll review that Monday when I'm back at it. Might be doable.

Thanks.




Brian

Re: Some help for a C++ guy

[tmoran]

>generic
>package Message_Handler is
>   procedure Message_Callback (Status : in M1553.IO_Status_T);
>...
>   function Num_Message_Callbacks return OS_Types.Unsigned_Word;
>...
>  Data_Buf_Ptr : constant M1553.Data_Buf_VPtr_T := new M1553.Data_Buf;
>  Stat_Buf_Ptr : constant M1553.Status_Buf_VPtr_T := new M1553.Status_Buf_T;
>...
>Then in the implementation code, there's stuff like:
>
>   package Message_0_Handler is new Message_Handler;
>   package Message_5_Handler is new Message_Handler;
>...
>I don't much like this. It's cumbersome even for a couple of messages,
>let alone when we grow to 20 or more, because we have duplicated code
>for each registration.
>
>What I'd like is to have some sort of array of the messages. That way
>for errors. I guess I'm thinking an OO approach, as long as the various
>members could be passed as arguments to the registration routines.
  Since the generic has no parameters, what you're getting is in effect
a single body of code, with separate data for each instantiation, and
you call
  Message_0_Handler.Message_Callbacks (Status);
instead of
  Message_Handler.Message_Callbacks (Message_0, Status);

So you could just drop the "generic" and put the package's local data
into a record:

 package Message_Handler is

    type Message_State_Type is private;

    procedure Message_Callback (State  : in Message_State_Type;
                                Status : in M1553.IO_Status_T);
 ...
    function Num_Message_Callbacks (State  : in Message_State_Type)
    return OS_Types.Unsigned_Word;
 ...
    -- make Data_Buf_Ptr readable, but not writeable
    function Data_Buf_Ptr (State  : in Message_State_Type)
    return M1553.Data_Buf_VPtr_T;

    function Stat_Buf_Ptr (State  : in Message_State_Type)
    return M1553.Status_Buf_VPtr_T;
 ...

Then you could have
    Message_List : array(0 .. 5) of Message_State_Type;
and call
    Message_Handler.Message_Callback (Message_List(0), Status);
    etc
That seems the most vanilla change, and you can probably change all
your source code by a simple macro in your editor.
Or am I missing something?

Re: Some help for a C++ guy

[defaultuserbr]

tmoran@acm.org wrote:

> >What I'd like is to have some sort of array of the messages. That
way
> >for errors. I guess I'm thinking an OO approach, as long as the
various
> >members could be passed as arguments to the registration routines.
>   Since the generic has no parameters, what you're getting is in
effect
> a single body of code, with separate data for each instantiation, and
> you call
>   Message_0_Handler.Message_Callbacks (Status);
> instead of
>   Message_Handler.Message_Callbacks (Message_0, Status);
>
> So you could just drop the "generic" and put the package's local data
> into a record:

To tell the truth, there probably should be some sort of parameter.
Each message has a unique message ID that really should be carried in
the package/class/whatever.

>  package Message_Handler is
>
>     type Message_State_Type is private;
>
>     procedure Message_Callback (State  : in Message_State_Type;
>                                 Status : in M1553.IO_Status_T);
>  ...
>     function Num_Message_Callbacks (State  : in Message_State_Type)
>     return OS_Types.Unsigned_Word;
>  ...
>     -- make Data_Buf_Ptr readable, but not writeable
>     function Data_Buf_Ptr (State  : in Message_State_Type)
>     return M1553.Data_Buf_VPtr_T;
>
>     function Stat_Buf_Ptr (State  : in Message_State_Type)
>     return M1553.Status_Buf_VPtr_T;
>  ...
>
> Then you could have
>     Message_List : array(0 .. 5) of Message_State_Type;
> and call
>     Message_Handler.Message_Callback (Message_List(0), Status);
>     etc
> That seems the most vanilla change, and you can probably change all
> your source code by a simple macro in your editor.
> Or am I missing something?

Ok, I'll take a look at this. Thanks for taking the time to write this
up. I know it's somewhat frustrating to have people who don't really
know your language that well come in with some problem to solve, people
who can't barely articulate what they want to do and what problems they
are having. I appreciate all the input so far.



Brian

Re: Some help for a C++ guy

[Matthew Heaney]

defaultuserbr@yahoo.com writes:

> To tell the truth, there probably should be some sort of parameter.
> Each message has a unique message ID that really should be carried in
> the package/class/whatever.

Can you move the state data into the handler?

package P is

   type Handler_Type is abstract tagged limited null record;

   procedure Process_Status
     (Handler : in out Handler_Type;
      Status  : in     Status_Type) is abstract;

   procedure Register (Handler : in Handler_Type);

...
end P;

Clients derive from Handler_Type and add state as necessary.