Re: [AD] Ouya controller support

[Peter Wang <novalazy@xxxxxxxxxx>]

On Sat, 13 Jul 2013 18:06:37 -0600, Trent Gamblin <nooskewl@xxxxxxxxxx> wrote:
> If the idea is ok, here’s a patch. I only tested it on Ouya and not desktop Linux. I had to change the call to inotify_init1 to inotify_init with an fcntl since Ouya doesn’t have that function or constant (IN_NONBLOCK.)
> 


> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 347465d..627c86c 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -443,7 +443,6 @@ check_include_files(machine/soundcard.h ALLEGRO_HAVE_MACHINE_SOUNDCARD_H)
> check_include_files(linux/soundcard.h ALLEGRO_HAVE_LINUX_SOUNDCARD_H)
> check_include_files(libkern/OSAtomic.h ALLEGRO_HAVE_OSATOMIC_H)
> check_include_files(sys/inotify.h ALLEGRO_HAVE_SYS_INOTIFY_H)
> -check_include_files(sys/timerfd.h ALLEGRO_HAVE_SYS_TIMERFD_H)
> 
> check_function_exists(getexecname ALLEGRO_HAVE_GETEXECNAME)
> check_function_exists(mkstemp ALLEGRO_HAVE_MKSTEMP)
> diff --git a/include/allegro5/platform/alplatf.h.cmake b/include/allegro5/platform/alplatf.h.cmake
> index 9c26764..c7d41a7 100644
> --- a/include/allegro5/platform/alplatf.h.cmake
> +++ b/include/allegro5/platform/alplatf.h.cmake
> @@ -49,7 +49,6 @@
> #cmakedefine ALLEGRO_HAVE_SYS_TYPES_H
> #cmakedefine ALLEGRO_HAVE_OSATOMIC_H
> #cmakedefine ALLEGRO_HAVE_SYS_INOTIFY_H
> -#cmakedefine ALLEGRO_HAVE_SYS_TIMERFD_H
> 
> /* Define to 1 if the corresponding functions are available. */
> #cmakedefine ALLEGRO_HAVE_GETEXECNAME
> diff --git a/src/linux/ljoynu.c b/src/linux/ljoynu.c
> index 928640e..97668a5 100644
> --- a/src/linux/ljoynu.c
> +++ b/src/linux/ljoynu.c
> @@ -42,10 +42,9 @@
> #include <sys/types.h>
> #include <linux/input.h>
> 
> -#if defined(ALLEGRO_HAVE_SYS_INOTIFY_H) && defined(ALLEGRO_HAVE_SYS_TIMERFD_H)
> +#if defined(ALLEGRO_HAVE_SYS_INOTIFY_H)
>     #define SUPPORT_HOTPLUG
>     #include <sys/inotify.h>
> -   #include <sys/timerfd.h>
> #endif
> 
> ALLEGRO_DEBUG_CHANNEL("ljoy");
> @@ -127,9 +126,34 @@ static volatile bool config_needs_merging;
> static ALLEGRO_MUTEX *config_mutex;
> #ifdef SUPPORT_HOTPLUG
> static int inotify_fd = -1;
> -static int timer_fd = -1;
> -#endif
> 
> +static ALLEGRO_THREAD *hotplug_thread;
> +static ALLEGRO_MUTEX *hotplug_mutex;
> +static ALLEGRO_COND *hotplug_cond;
> +static bool hotplug_ended = false;
> +static void ljoy_scan(bool configure);
> +
> +static void *hotplug_proc(ALLEGRO_THREAD *thread, void *data)
> +{
> +   (void)data;
> +
> +   while (!al_get_thread_should_stop(thread)) {
> +      if (!hotplug_ended) {
> +         al_wait_cond(hotplug_cond, hotplug_mutex);
> +      }
> +
> +      al_rest(1);
> +
> +      al_lock_mutex(config_mutex);
> +      ljoy_scan(true);
> +      al_unlock_mutex(config_mutex);
> +   }
> +
> +   hotplug_ended = false;
> +
> +   return NULL;
> +}
> +#endif

Eh, I hate it when these FOTM devices start impacting the code base.

hotplug_ended should be tested after the al_wait_cond so there is not
a one second delay & rescan when shutting down.

> /* Return true if a joystick-related button or key:
> @@ -595,7 +619,6 @@ static void ljoy_merge(void)
> static void ljoy_config_dev_changed(void *data)
> {
>     char buf[128];
> -   struct itimerspec spec;
>     (void)data;
> 
>     /* Empty the event buffer. We only care that some inotify event was sent but it
> @@ -605,33 +628,7 @@ static void ljoy_config_dev_changed(void *data)
>     while (read(inotify_fd, buf, sizeof(buf)) > 0) {
>     }
> 
> -   /* Set the timer to fire once in one second.
> -    * We cannot scan immediately because the devices may not be ready yet :-P
> -    */
> -   spec.it_value.tv_sec = 1;
> -   spec.it_value.tv_nsec = 0;
> -   spec.it_interval.tv_sec = 0;
> -   spec.it_interval.tv_nsec = 0;
> -   timerfd_settime(timer_fd, 0, &spec, NULL);
> -}
> -
> -
> -
> -/* ljoy_config_rescan: [fdwatch thread]
> - *  Rescans for joystick devices a little while after devices change.
> - */
> -static void ljoy_config_rescan(void *data)
> -{
> -   uint64_t exp;
> -   (void)data;
> -
> -   /* Empty the event buffer. */
> -   while (read(timer_fd, &exp, sizeof(uint64_t)) > 0) {
> -   }
> -
> -   al_lock_mutex(config_mutex);
> -   ljoy_scan(true);
> -   al_unlock_mutex(config_mutex);
> +   al_signal_cond(hotplug_cond);
> }
> #endif
> 
> @@ -652,13 +649,12 @@ static bool ljoy_init_joystick(void)
>     config_mutex = al_create_mutex();
> 
> #ifdef SUPPORT_HOTPLUG
> -   inotify_fd = inotify_init1(IN_NONBLOCK);
> -   timer_fd = timerfd_create(CLOCK_REALTIME, TFD_NONBLOCK);
> -   if (inotify_fd != -1 && timer_fd != -1) {
> +   inotify_fd = inotify_init();
> +   if (inotify_fd != -1) {
> +      fcntl(inotify_fd, F_SETFL, O_NONBLOCK);
>        /* Modern Linux probably only needs to monitor /dev/input. */
>        inotify_add_watch(inotify_fd, "/dev/input", IN_CREATE|IN_DELETE);
>        _al_unix_start_watching_fd(inotify_fd, ljoy_config_dev_changed, NULL);
> -      _al_unix_start_watching_fd(timer_fd, ljoy_config_rescan, NULL);
>        ALLEGRO_INFO("Hotplugging enabled\n");
>     }
>     else {
> @@ -667,11 +663,12 @@ static bool ljoy_init_joystick(void)
>           close(inotify_fd);
>           inotify_fd = -1;
>        }
> -      if (timer_fd != -1) {
> -         close(timer_fd);
> -         timer_fd = -1;
> -      }
>     }
> +
> +   hotplug_mutex = al_create_mutex();
> +   hotplug_cond = al_create_cond();
> +   hotplug_thread = al_create_thread(hotplug_proc, NULL);
> +   al_start_thread(hotplug_thread);
> #endif

Please do error checking here.

Can you move hotplug_thread to roughly where the other hotplug functions
are currently?  Thanks.

Peter