Serial: Initial refactor

Serial: Initial refactor

[Okash Khawaja]

Hi Samuel,

>From previous email:

"You will notice calls to spk_serial_out() in spk_do_catch_up() and
spk_synth_flush(). Actually the very first step of your work could be
to add a serial_out() method to drivers, which for now would be set
to spk_serial_out() in all drivers, and make spk_do_catch_up() and
spk_synth_flush() call the method instead of spk_serial_out(). Direct
calls to spk_serial_out() within drivers could be converted into calling
the method too, so that switching methods will be trivial."

So I am wondering if it will be possible to restrict the changes to
speakup_dummy while we test the new implementation using tty->ops->write?
That means a transitional phase where we have both, the existing serial io
for other drivers and new implementation for dummy driver. Or is that what
you meant?

Thanks,
Okash

Re: Serial: Initial refactor

[Samuel Thibault]

Okash Khawaja, on Fri 18 Nov 2016 07:07:33 +0000, wrote:
> "You will notice calls to spk_serial_out() in spk_do_catch_up() and
> spk_synth_flush(). Actually the very first step of your work could be
> to add a serial_out() method to drivers, which for now would be set
> to spk_serial_out() in all drivers, and make spk_do_catch_up() and
> spk_synth_flush() call the method instead of spk_serial_out(). Direct
> calls to spk_serial_out() within drivers could be converted into calling
> the method too, so that switching methods will be trivial."
> 
> So I am wondering if it will be possible to restrict the changes to
> speakup_dummy while we test the new implementation using tty->ops->write? That
> means a transitional phase where we have both, the existing serial io for other
> drivers and new implementation for dummy driver. Or is that what you meant?

That's what I meant :)

Samuel

Re: Serial: Initial refactor

[Okash Khawaja]

Cool. So we start with spk_serial_out() by replacing it with a wrapper
wherever it is used in speakup_dummy?

For first pass, here's what I am thinking (from your suggestion but using
uglier function names which parallel existing ones):

- add spk_serial_out2() wrapper which delegates to spk_serial_out()
- add spk_synth_flush2() which is same as spk_synth_flush() but calls
spk_serial_out2() instead
- add spk_do_catch_up2() which is same as spk_do_catch_up() but calls
spk_serial_out2() instead
- in speakup_dummy replace calls to above functions with their *2()
alternative

Is that fine?

Thanks,
Okash

On Fri, Nov 18, 2016 at 7:44 AM, Samuel Thibault <
samuel.thibault@ens-lyon.org> wrote:

> Okash Khawaja, on Fri 18 Nov 2016 07:07:33 +0000, wrote:
> > "You will notice calls to spk_serial_out() in spk_do_catch_up() and
> > spk_synth_flush(). Actually the very first step of your work could be
> > to add a serial_out() method to drivers, which for now would be set
> > to spk_serial_out() in all drivers, and make spk_do_catch_up() and
> > spk_synth_flush() call the method instead of spk_serial_out(). Direct
> > calls to spk_serial_out() within drivers could be converted into calling
> > the method too, so that switching methods will be trivial."
> >
> > So I am wondering if it will be possible to restrict the changes to
> > speakup_dummy while we test the new implementation using
> tty->ops->write? That
> > means a transitional phase where we have both, the existing serial io
> for other
> > drivers and new implementation for dummy driver. Or is that what you
> meant?
>
> That's what I meant :)
>
> Samuel
>

Re: Serial: Initial refactor

[Samuel Thibault]

Okash Khawaja, on Fri 18 Nov 2016 09:20:08 +0000, wrote:
> Cool. So we start with spk_serial_out() by replacing it with a wrapper wherever
> it is used in speakup_dummy?

By replacing it with calling serial_out method, i.e. call synth->serial_out.
And in all drivers for now, use spk_serial_out for that method.

Then you can write an spk_serial_out2 which just calls spk_serial_out,
and make the dummy driver use that for the serial_out method instead of
spk_serial_out.

> - add spk_synth_flush2() which is same as spk_synth_flush() but calls
> spk_serial_out2() instead
> - add spk_do_catch_up2() which is same as spk_do_catch_up() but calls
> spk_serial_out2() instead

No, we don't want to define new functions for these, just make them call
the serial_out of the synth.

Samuel

Re: Serial: Initial refactor

[Okash Khawaja]

Got it, thanks


> On 18 Nov 2016, at 12:39, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> 
> Okash Khawaja, on Fri 18 Nov 2016 09:20:08 +0000, wrote:
>> Cool. So we start with spk_serial_out() by replacing it with a wrapper wherever
>> it is used in speakup_dummy?
> 
> By replacing it with calling serial_out method, i.e. call synth->serial_out.
> And in all drivers for now, use spk_serial_out for that method.
> 
> Then you can write an spk_serial_out2 which just calls spk_serial_out,
> and make the dummy driver use that for the serial_out method instead of
> spk_serial_out.
> 
>> - add spk_synth_flush2() which is same as spk_synth_flush() but calls
>> spk_serial_out2() instead
>> - add spk_do_catch_up2() which is same as spk_do_catch_up() but calls
>> spk_serial_out2() instead
> 
> No, we don't want to define new functions for these, just make them call
> the serial_out of the synth.
> 
> Samuel

Re: Serial: Initial refactor

[David]

Just add a few #defines in an #include file and there is no code
change. #define spk_serial_out spk_serial_out2 etc.

Re: Serial: Initial refactor

[Okash Khawaja]

Hi David,

Thanks. I'm personally averse to #defines although open to discuss.

Okash


> On 18 Nov 2016, at 14:43, David <david.a.borowski@gmail.com> wrote:
> 
> Just add a few #defines in an #include file and there is no code
> change. #define spk_serial_out spk_serial_out2 etc.

Re: Serial: Initial refactor

[Samuel Thibault]

David, on Fri 18 Nov 2016 09:43:28 -0500, wrote:
> Just add a few #defines in an #include file and there is no code
> change. #define spk_serial_out spk_serial_out2 etc.

It makes the code more complex for no benefit.

Changing the code is cheap. Making the code more complex costs a lot on
the long run.

Samuel