TypedControls branch? was Re: [Clam-devel] Bug solved for deleting Out In Ports and Out In Controls

Hernán Ordiales h at ordia.com.ar
Thu Jul 17 10:10:04 PDT 2008


On Thu, Jul 17, 2008 at 1:19 PM, Pau Arumí <parumi at iua.upf.edu> wrote:
> On dj, 2008-07-17 at 12:56 -0300, Hernán Ordiales wrote:
>> On Thu, Jul 17, 2008 at 12:39 PM, Pau Arumí <parumi at iua.upf.edu> wrote:
>> > On dj, 2008-07-17 at 10:49 -0300, Francisco Tufró wrote:
>> >> Hi, if we take this as a bug/feature that needs to be implemented, it
>> >> also needs to be implemented in typed controls.
>> >> I'll add this to the next patch.
>> >
>> > Hi Francisco,
>> > TypedIn|OutControl<T> derives from In|OutControl, right? In that case
>> > there is nothing to be done. It is all base-class behavior.
>>
>> nope, in the current patch implementation there is a
>> BaseTypedIn|OutControl and In|OutControl inherits from
>> TypedIn|OutControl<TControlData>, anyway i think is just a matter to
>> update the code related to BaseTypedIn|OutControl in the same way
>> Ferrarn did.
>
> ah, yes. BaseTyped*Control have CanConnect() interface. Yes synching
> that patch is not a problem.
>
>>  BTW, did you checked the last mails of the thread:
>> "TypedControls: asking for advice and opinions"? new problems with
>> some internal implementations (needed for a complete migration) arose
>> and we were not completely sure about the solutions of some issues...
>
> I was not sure if all was addressed or not. Could your or Francisco send
> a brief list of pending issues to be addressed?

below i'm copying my last mail in response to a Francisco's one where
i think each issue is listed...

>> Francisco: please send/resend the patch to the list (although could be
>> incomplete at the moment), just to have a better picture of what we
>> are talking about...
>
> The TypedControls stuff has shown to be much more complicated and have
> many ramifications than initially planed. I fear that managing this big
> work with patches is not efficient. On the same time, we are dealing
> with base class substitutions and so we can not commit till the whole
> port is complete.

I'm absolutely agree.

> Therefore, I see all the ingredients to work with a branch (a
> not-keen-about-subversion-branches person is speaking)

Me too. Working this with patches is getting more complex every day
(the patch increases since can't be committed yet and it's also hard
to work on it by more that one person and maintain it updated)

> >From some weeks ago I'm trying to have a launchpad/bazaar CLAM svn
> import, but it is proving difficult:
> https://code.launchpad.net/clam

i'm not sure how this is supposed to work :(

> So I propose to create a branch in our subversion. This branch should
> live only during the controls transition, and the maintainers should
> keep all the changes in trunk commited in the branch. That is, in
> permanent state of merge with trunk.
>
> What do you think?

I really like the idea. How we continue with this?


My previously mentioned mail:

Hi all,

On Fri, Jul 11, 2008 at 3:12 PM, Francisco Tufró <nictuku at gmail.com> wrote:
> Hi, I've been trying to do the migration to TypedControls and we've some
> problems.
> Since the *ControlRegistry now is defined as std::list<BaseTyped*Control*>
> the functions Get() and GetByNumber now return a BaseTyped*Control instead
> of a *Control.
> The same is happening with Get*Controls() which now returns a list<Base..>
> instead of a list<*Control *>
> So, every single code line using Get*Controls as well as Get() and
> GetByNumber() has to be replaced.
> As Pau proposed i wrote the function that connected a In to a temporary out
> and sends the value, but that was just one of the appearances of the
> problem.
[snip]
>  - For the already known problem (the one of this thread) i wrote the
> following function
> (i made also a version where an int is passed instead of the name, but is
> essentially the same, for those places where GetByNumber was used):
>     void SendFloatToInControl(Processing & receiver, const std::string &
> inControlName, float value){
>         TypedOutControl<float> controlSender;
>         controlSender.AddLink(receiver.GetInControls().Get(inControlName));
>         controlSender.SendControl(value);
>     }

> I tried to solve every one of these problems with the better solution i
> thought, now i present these solutions:

I'll reorder the problems you exposed and proposed solutions:

> Those functiones were used in other circumstances:
>   - Assignations: MIDIIO/MIDIKeyboard.cxx:        OutControl& outVelocity =
> mNoteIn.GetOutControls().GetByNumber(2);

> The first i had to do a static cast, i don't like this solution, but the
> code needs to be rewritten to avoid it, this happends also in NetworkEditor
> and Annotator.

This seems the more ugly...

>   - To access in controls defined in parent classes:
> SpectralSpread.hxx:
> GetInControl("Amount").DoControl(100);

> The second was solved with this function also, but passed *this as argument.

ok, but if controls are protected (or public) i think you can use the
var name instead (like third case) and if not, i think they should...

>   - also (don't know why) in the Initialization function of some processings
> (where the controls were accesible by its own variable name, but anyways
> they were referenced with Get*Controls:
> FrameTransformation.hxx:
> GetInControl("Amount").DoControl(0.);

> The third was solved replacing the lines using the variable name.

ok for me.

> Another of these problems was that GetInControl was used to ask for
> GetLastValue of an InControl, for this i wrote:
> float GetFloatFromInControl(Processing & proc, const std::string &
> inControlName){
>         InControl& in = (InControl&)(proc.GetInControl(inControlName));
>         return in.GetLastValue();
>     }
> This needed a typecast, but static. I'm not so sure about this solution.
> This was needed in the SMSDeesser lines 44-47.

ok

> Also there were some places that used GetOutControl to do a SendControl,
> with these i used this function:
>
>     void SendFloatToOutControl(Processing & sender, int inControlIndex,
> float value){
>         OutControl& out =
> (OutControl&)(sender.GetOutControls().GetByNumber(inControlIndex));
>         out.SendControl(value);
>     }
> Again, a cast was needed, used in MIDIInControl.hxx.

ok

> Hope you can help me with this and tell me how to best solve the migration
> problem.
> bye!

Well, actually we already discussed those problems in private, now I'm
interested to hear Pau and David opinions about all these cases since
are very close to the clam core and I'd like to know if have better
ideas or think the patch is ready for commitment...
Franciso's patch with these solutions compiles[1] and pass all tests
and he was already playing a bit with networks at a (patched) NE and
clam lib.

[1] at least NE and clam lib, I think is pending a similar review to
annotator and smstools but i hope that no new use case emerge

cheers

-- 
Hernán
http://h.ordia.com.ar
GnuPG: 0xEE8A3FE9




More information about the clam-devel mailing list