Hi,
On Mon, Feb 20, 2017 at 12:57 PM, Samuel Thibault
<samuel.thibault@xxxxxxxxxxxx> wrote:
> Hello,
>
> Okash Khawaja, on lun. 20 févr. 2017 12:23:45 +0000, wrote:
>> > On 19 Feb 2017, at 23:06, Samuel Thibault <samuel.thibault@xxxxxxxxxxxx> wrote:
>> >
>> > Okash Khawaja, on dim. 19 févr. 2017 10:44:36 +0000, wrote:
>> >> Some drivers like apollo and bns only assign synth_immediate method in
>> >> their spk_synth structs but don't seem to use it. For those drivers,
>> >> can it be set to NULL?
>> >
>> > I guess this method could be useful for some screen reading features,
>> > just not used yet.
>>
>> I see. Now because they are assigned but not used, they may cause confusion. For example, spk_synth_immediate uses outb.
>
> Oh, I didn't realize this.
>
>> When migrating a synth to ttyio, if synth_immediate is left assigned, it may look like that synth isn't fully migrated yet.
>
> Indeed, and that can only lead to problems later.
>
>> We can leave as it is although I would prefer it to be set to NULL
>> until it is actually used.
>
> Indeed.
>
> Ideally however we'd have a ttyio variant to assign here. It shouldn't
> be hard, something like this (untested):
>
> const char *spk_ttyio_synth_immediate(struct spk_synth *synth, const char *buff)
> {
> u_char ch;
>
> while ((ch = *buff)) {
> if (ch == '\n')
> ch = synth->procspeech;
> if (tty_write_room(speakup_tty) < 1 || !spk_ttyio_out(ch))
> return buff;
> buff++;
> }
> return NULL;
> }
I've already done this for speakup_acntsa.
>
> And the existing spk_synth_immediate should be renamed to
> spk_serial_synth_immediate to make it clear it uses I/O ports.
Sure, and I'll also move it into serialio.c from your comments below.
>
> Thinking about this, spk_ttyio_out should check the result of the write
> operation, something like this:
Good point. Will update the code.
>
> int spk_ttyio_out(const char ch)
> {
> if (synth->alive && speakup_tty && speakup_tty->ops->write) {
> int ret = speakup_tty->ops->write(speakup_tty, &ch, 1);
> if (ret == 0)
> /* No room */
> return 0;
> if (ret < 0) {
> pr_warn("%s: I/O error, deactivating speakup\n", synth->long_name);
> /* No synth any more, so nobody will restart TTYs, and we thus
> * need to do it ourselves. Now that there is no synth we can
> * let application flood anyway
> */
> synth->alive = 0;
> speakup_start_ttys();
> return 0;
> }
> return 1;
> }
> return 0;
> }
>
> BTW, I can see that spk_serial_synth_probe uses outb too, it should be
> moved to serialio.c too. In the end, we should have a situation where
> only serialio.c functions use in[bwl]/out[bwl], and these functions be
> prefixed with spk_serial_, so that it's easy to know which code parts
> still use them.
Cool, will send in the patch set.
Okash
_______________________________________________
Speakup mailing list
Speakup@xxxxxxxxxxxxxxxxx
http://linux-speakup.org/cgi-bin/mailman/listinfo/speakup
