Re: [Qemu-devel] [PATCH] baum: Add support for typing ascii

[Peter Maydell]

On 30 August 2015 at 15:21, Samuel Thibault <address@hidden> wrote:
> This adds support for typing ascii through the Baum Braille driver, by
> translating it to braille with the NABCC table.
>
> Signed-off-by: Samuel Thibault <address@hidden>
>
> diff --git a/backends/baum.c b/backends/baum.c
> index a69aaff..d486e68 100644
> --- a/backends/baum.c
> +++ b/backends/baum.c
> @@ -1,7 +1,7 @@
>  /*
>   * QEMU Baum Braille Device
>   *
> - * Copyright (c) 2008, 2015 Samuel Thibault
> + * Copyright (c) 2008, 2015 Samuel Thibault
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -474,6 +474,13 @@ static void baum_send_key(BaumDriverState *baum, uint8_t 
> type, uint8_t value) {
>      baum_write_packet(baum, packet, sizeof(packet));
>  }
>
> +/* Send the 2-byte key code to the other end */
> +static void baum_send_key2(BaumDriverState *baum, uint8_t type, uint16_t 
> value) {
> +    uint8_t packet[] = { type, value & 0xFF, value >> 8 };
> +    DPRINTF("writing key %x %x\n", type, value);
> +    baum_write_packet(baum, packet, sizeof(packet));
> +}
> +
>  /* We got some data on the BrlAPI socket */
>  static void baum_chr_read(void *opaque)
>  {
> @@ -492,6 +499,14 @@ static void baum_chr_read(void *opaque)
>                  baum_send_key(baum, BAUM_RSP_RoutingKey, (code & 
> BRLAPI_KEY_CMD_ARG_MASK)+1);
>                  baum_send_key(baum, BAUM_RSP_RoutingKey, 0);
>                  break;
> +            case BRLAPI_KEY_CMD_PASSDOTS:
> +                {
> +                    unsigned char dots = code & BRLAPI_KEY_CMD_ARG_MASK;
> +                    fprintf(stderr,"passdots %x\n", dots);

Should this be a DPRINTF ?

> +                    baum_send_key2(baum, BAUM_RSP_EntryKeys, dots << 8);
> +                    baum_send_key2(baum, BAUM_RSP_EntryKeys, 0);
> +                    break;
> +                }
>              case 0:
>                  switch (code & BRLAPI_KEY_CMD_ARG_MASK) {
>                  case BRLAPI_KEY_CMD_FWINLT:
> @@ -538,7 +553,27 @@ static void baum_chr_read(void *opaque)
>              }
>              break;
>          case BRLAPI_KEY_TYPE_SYM:
> -            break;
> +            {
> +                unsigned modifiers = ((code & BRLAPI_KEY_FLAGS_MASK) >> 
> BRLAPI_KEY_FLAGS_SHIFT) & 0xFF;
> +                unsigned keysym = code & BRLAPI_KEY_CODE_MASK;
> +                unsigned dots;
> +                if (modifiers & ~1)
> +                    /* Unsupported */
> +                    break;
> +                if (keysym <= ' ' || keysym > '~')
> +                    /* Unsupported */
> +                    break;

QEMU coding style wants braces for all if statements, even with single
line bodies. (Try scripts/checkpatch.pl.)

> +                DPRINTF("keysym %x\n", keysym);
> +                for (dots = 1; dots <= 0xFF; dots++)

This for () needs braces too.

> +                    if (nabcc_translation[dots] == keysym)
> +                    {
> +                        DPRINTF("dots %x\n", dots);
> +                        baum_send_key2(baum, BAUM_RSP_EntryKeys, dots << 8);
> +                        baum_send_key2(baum, BAUM_RSP_EntryKeys, 0);
> +                        break;
> +                    }

Does this happen often enough to make a glib hashtable preferable
to the linear scan through a 256-entry array ?

> +                break;
> +            }
>          }
>      }
>      if (ret == -1 && (brlapi_errno != BRLAPI_ERROR_LIBCERR || errno != 
> EINTR)) {
>

thanks
-- PMM