Revamped how volume changes are handled to also support remote volume control

software/main/CMakeLists.txt

@@ -1,3 +1,3 @@
-idf_component_register(SRCS "src/main.cpp" "src/helper.cpp" "src/wav.cpp" "src/storage.cpp" "src/i2s.cpp" "src/bluetooth.cpp" "src/avrcp.cpp" "src/a2dp.cpp" "src/twai.cpp"
+idf_component_register(SRCS "src/main.cpp" "src/helper.cpp" "src/wav.cpp" "src/storage.cpp" "src/i2s.cpp" "src/bluetooth.cpp" "src/avrcp.cpp" "src/a2dp.cpp" "src/twai.cpp" "src/volume.cpp"
                     INCLUDE_DIRS "include"
                     EMBED_FILES "assets/connect.wav" "assets/disconnect.wav")

software/main/include/twai.h

@@ -2,5 +2,6 @@
 
 namespace twai {
 	void init();
+	void change_volume(bool up);
 }
 

software/main/include/volume.h

@@ -0,0 +1,12 @@
+#pragma once
+
+#include <cstdint>
+#include <sys/lock.h>
+
+namespace volume_controller {
+	void init();
+	void set_from_radio(int volume);
+	void set_from_remote(int volume);
+
+	uint8_t current();
+}

software/main/src/avrcp.cpp

@@ -2,6 +2,7 @@
 #include "esp_avrc_api.h"
 
 #include "avrcp.h"
+#include "volume.h"
 #include "helper.h"
 
 #define AVRCP_TAG "APP_AVRCP"
@@ -9,8 +10,6 @@
 static esp_avrc_rn_evt_cap_mask_t s_avrc_peer_rn_cap;
 static esp_avrc_playback_stat_t playback_status = ESP_AVRC_PLAYBACK_STOPPED;
 
-static uint8_t volume = 127;
-static _lock_t volume_lock;
 static bool volume_notify = false;
 
 static void playback_changed() {
@@ -33,13 +32,6 @@ static void notify_handler(uint8_t event_id, esp_avrc_rn_param_t *event_paramete
 	}
 }
 
-static void set_volume_value(uint8_t v) {
-	ESP_LOGI(AVRCP_TAG, "Setting internal volume value to %i", v);
-	_lock_acquire(&volume_lock);
-	volume = v;
-	_lock_release(&volume_lock);
-}
-
 static void rc_ct_callback(esp_avrc_ct_cb_event_t event, esp_avrc_ct_cb_param_t *param) {
 	ESP_LOGD(AVRCP_TAG, "%s event %d", __func__, event);
 
@@ -81,8 +73,7 @@ static void rc_tg_callback(esp_avrc_tg_cb_event_t event, esp_avrc_tg_cb_param_t*
 	switch (event) {
 		case ESP_AVRC_TG_SET_ABSOLUTE_VOLUME_CMD_EVT:
 			ESP_LOGI(AVRCP_TAG, "AVRC set absolute volume: %d%%", (int)param->set_abs_vol.volume * 100/ 0x7f);
-			// The phone want to change the volume
+			volume_controller::set_from_remote(param->set_abs_vol.volume);
-			set_volume_value(param->set_abs_vol.volume);
 			break;
 
 		case ESP_AVRC_TG_REGISTER_NOTIFICATION_EVT:
@@ -97,7 +88,7 @@ static void rc_tg_callback(esp_avrc_tg_cb_event_t event, esp_avrc_tg_cb_param_t*
 				// This will require some sort of CAN bus access
 				volume_notify = true;
 				esp_avrc_rn_param_t rn_param;
-				rn_param.volume = volume;
+				rn_param.volume = volume_controller::current();
 				esp_avrc_tg_send_rn_rsp(ESP_AVRC_RN_VOLUME_CHANGE, ESP_AVRC_RN_RSP_INTERIM, &rn_param);
 			} else if (param->reg_ntf.event_id == ESP_AVRC_RN_PLAY_POS_CHANGED) {
 				ESP_LOGI(AVRCP_TAG, "We can change the play position?");
@@ -112,6 +103,14 @@ static void rc_tg_callback(esp_avrc_tg_cb_event_t event, esp_avrc_tg_cb_param_t*
 	}
 }
 
+static void send_cmd(esp_avrc_pt_cmd_t cmd) {
+	esp_err_t ret = esp_avrc_ct_send_passthrough_cmd(0, cmd, ESP_AVRC_PT_CMD_STATE_PRESSED);
+	ESP_ERROR_CHECK(ret);
+
+	ret = esp_avrc_ct_send_passthrough_cmd(1, cmd, ESP_AVRC_PT_CMD_STATE_RELEASED);
+	ESP_ERROR_CHECK(ret);
+}
+
 void avrcp::init() {
 	ESP_LOGI(AVRCP_TAG, "Initializing AVRCP");
 
@@ -147,14 +146,6 @@ bool avrcp::is_playing() {
 	return playback_status == ESP_AVRC_PLAYBACK_PLAYING;
 }
 
-static void send_cmd(esp_avrc_pt_cmd_t cmd) {
-	esp_err_t ret = esp_avrc_ct_send_passthrough_cmd(0, cmd, ESP_AVRC_PT_CMD_STATE_PRESSED);
-	ESP_ERROR_CHECK(ret);
-
-	ret = esp_avrc_ct_send_passthrough_cmd(1, cmd, ESP_AVRC_PT_CMD_STATE_RELEASED);
-	ESP_ERROR_CHECK(ret);
-}
-
 void avrcp::play() {
 	ESP_LOGI(AVRCP_TAG, "Playing");
 	send_cmd(ESP_AVRC_PT_CMD_PLAY);
@@ -198,18 +189,10 @@ void avrcp::seek_backward() {
 }
 
 void avrcp::set_volume(uint8_t v) {
-	// Make sure the volume is actually changed
-	if (v == volume) {
-		return;
-	}
-
-	set_volume_value(v);
-
-	// @TODO What we the device supports remote volume but we can not send it yet?
 	if (volume_notify) {
 		ESP_LOGI(AVRCP_TAG, "Setting remote volume value to %i", v);
 		esp_avrc_rn_param_t rn_param;
-		rn_param.volume = volume;
+		rn_param.volume = v;
 		esp_avrc_tg_send_rn_rsp(ESP_AVRC_RN_VOLUME_CHANGE, ESP_AVRC_RN_RSP_CHANGED, &rn_param);
 		volume_notify = false;
 	}

software/main/src/twai.cpp

@@ -10,6 +10,7 @@
 #include "twai.h"
 #include "avrcp.h"
 #include "can_data.h"
+#include "volume.h"
 #include "helper.h"
 
 #define TWAI_TAG "APP_TWAI"
@@ -47,6 +48,40 @@ static void send_test() {
 	}
 }
 
+// @TODO Make sure that the other buttons are set to match the current state
+// That way way the scroll wheel and long pressing will not have unintented effects
+void twai::change_volume(bool up) {
+	can::Buttons buttons;
+	memset(&buttons, 0, sizeof(buttons));
+
+	if (up) {
+		buttons.volume_up = true;
+	} else {
+		buttons.volume_down = true;
+	}
+
+
+	twai_message_t message;
+	memset(&message, 0, sizeof(message));
+
+	message.identifier = BUTTONS_ID;
+	message.data_length_code = 3;
+	message.data[0] = ((uint8_t*)&buttons)[0];
+	message.data[1] = ((uint8_t*)&buttons)[1];
+	message.data[2] = ((uint8_t*)&buttons)[2];
+
+
+	for (int i = 0; i < message.data_length_code; i++) {
+		ESP_LOGI(TWAI_TAG, "%i: 0x%X", i, message.data[i]);
+	}
+
+	if (twai_transmit(&message, pdMS_TO_TICKS(1000)) == ESP_OK) {
+		ESP_LOGI(TWAI_TAG, "Message queued for transmission");
+	} else {
+		ESP_LOGI(TWAI_TAG, "Failed tp queue message for transmission");
+	}
+}
+
 static void listen(void*) {
 	for (;;) {
 		twai_message_t message;
@@ -86,7 +121,10 @@ static void listen(void*) {
 			case VOLUME_ID:
 				if (enabled) {
 					can::Volume volume = can::convert<can::Volume>(message.data, message.data_length_code);
-					avrcp::set_volume(ceil(volume.volume * 4.2f));
+					// Only update the volume if the volume has actually changed
+					if (!volume._1) {
+						volume_controller::set_from_radio(volume.volume);
+					}
 				}
 				break;
 

software/main/src/volume.cpp

@@ -0,0 +1,88 @@
+#include <cmath>
+
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "esp_log.h"
+
+#include "volume.h"
+#include "avrcp.h"
+#include "twai.h"
+
+#define VOLUME_TAG "APP_VOLUME"
+
+// 0-127
+static uint8_t volume;
+// 0-127
+static uint8_t remote_volume;
+// 0-30
+static uint8_t radio_volume;
+
+static bool synced = false;
+static _lock_t lock;
+
+void volume_controller::set_from_radio(int v) {
+	ESP_LOGI(VOLUME_TAG, "Volume on radio updated: %i (0-30)", v);
+	// Update the radio volume
+	_lock_acquire(&lock);
+	radio_volume = v;
+	_lock_release(&lock);
+
+	if (!synced) {
+		ESP_LOGI(VOLUME_TAG, "Not updating internal and remote (SYNCING)");
+		// In this case we are still adjusting the volume of the car to match the remote/internal volume
+		// So we do not want to update these values based on the radio
+		return;
+	}
+
+	// Convert the 0 - 30 range of the radio to 0 - 127
+	uint8_t full_range = ceil(v * 4.2f);
+	ESP_LOGI(VOLUME_TAG, "Updating internal and remote to: %i (0-127)", full_range);
+
+	// Update the remote volume
+	avrcp::set_volume(full_range);
+
+	// @TODO Somehow make sure we actually the remote volume is actually set before updating the value
+	_lock_acquire(&lock);
+	volume = full_range;
+	remote_volume = full_range;
+	_lock_release(&lock);
+}
+
+void volume_controller::set_from_remote(int v) {
+	ESP_LOGI(VOLUME_TAG, "Volume on remote updated: %i (0-127)", v);
+
+	_lock_acquire(&lock);
+	remote_volume = v;
+	volume = v;
+
+	synced = false;
+	_lock_release(&lock);
+}
+
+uint8_t volume_controller::current() {
+	return volume;
+}
+
+static void correct_volume(void*) {
+	for (;;) {
+		if (!synced) {
+			uint8_t target = floor(volume / 4.2f);
+
+			if (radio_volume == target) {
+				ESP_LOGI(VOLUME_TAG, "SYNCED!");
+				_lock_acquire(&lock);
+				synced = true;
+				_lock_release(&lock);
+			} else {
+				ESP_LOGI(VOLUME_TAG, "Adjusting volume: %i", radio_volume < target);
+				twai::change_volume(radio_volume < target);
+			}
+		}
+
+		vTaskDelay(pdMS_TO_TICKS(100));
+	}
+}
+
+void volume_controller::init() {
+	xTaskCreatePinnedToCore(correct_volume, "Correct volume", 2048, nullptr, 0, nullptr, 1);
+}