Thank you for taking the time to review my code.
On 4 Jan 2009, at 14:16, Takashi Iwai wrote:
At Sun, 4 Jan 2009 12:37:11 +0000, Peter Stokes wrote:
--- alsa-plugins-1.0.18-orig/arcam-av/arcam_av.c 1970-01-01 01:00:00.000000000 +0100 +++ alsa-plugins-1.0.18/arcam-av/arcam_av.c 2009-01-02 22:30:55.000000000 +0000
Looks like your embedded patches are broken due to line-breaks by MUA. Please fix it, or use attachments if it's difficult.
I'll try attaching it. Please find attached an updated patch.
+int arcam_av_connect(const char* port) +{
- int fd = open(port, O_RDWR | O_NOCTTY);
- if (fd < 0)
return -errno;- struct termios portsettings;
Put this into the beginning to follow the standard C.
Sorry, I am more used to programming in C++. I have attempted to move all of the variable declarations to the top of scope sections, but I may have missed some. I assume that was what you meant?
- if (bytes == 7)
return 0;- else if (bytes >= 0)
return -1;No proper error code?
I have modified this function to hopefully make it more robust.
+static int arcam_av_receive(int fd, arcam_av_cc_t* command, unsigned char* param1, unsigned char* param2) +{
- static int index = 0;
- static arcam_av_cc_t received_command;
- static unsigned char received_param1;
- static unsigned char received_param2;
Are static variables safe to use here? That is, is the plugin designed to access exclusively with a proper protection?
I believe so, yes.
The system is intended to broadly function as follows:
Each invocation of the plugin calls "arcam_av_connect()" to obtain a file descriptor for the serial device to be used to communicate with the amplifier. This file descriptor is used to send commands (requests to change state) directly to the amplifier (using the "arcam_av_send()" function). i.e. all invocations send directly to the amplifier.
Each invocation creates a server thread via the "arcam_av_server_start()" function. The server thread then switches into one of two state, master or slave. The server thread attempts to bind to a unix socket, if successful then there is currently no master server bound to the socket and the thread should become the master, if unsuccessful there is already a master bound to the socket and the thread should become a slave. The master server listens to the serial device for command responses from the amplifier (obtained using the "arcam_av_receive()" function). i.e. only the single master server receives directly from the amplifier.
Clients may connect to the master server via the unix socket (using the "arcam_av_client()" function). When the master server receives a successful command response from the amplifier it notifies all current clients that a change has occurred. This mechanism allows many clients (invocations of the plugin) to be notified of the change.
Slave servers also attach to the master server as a client. This connection is simply used to detect whether the master server has died as a result of the plugin invocation housing it being terminated. If the master server dies any slave servers attempt to bind to the unix socket and in doing so one would become the master server, any additional slaves would remain slaves.
Well, I'll review the rest after my vacation is over...
Thanks! Enjoy your vacation.
Peter