Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Joystick HID detection, add ADC Joystick mode, fix build on MacOS #408

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ radio/src/CmakeFiles/*
.push.settings.jsonc
build.sh
firm.sh
flash.sh
*.patch
8 changes: 2 additions & 6 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
cmake_minimum_required(VERSION 3.13)
project(OpenTX)

set(APPLE FALSE)
set(VERSION_MAJOR "1")
set(VERSION_MINOR "11")
set(VERSION_REVISION "1")
Expand All @@ -9,7 +11,6 @@ set(VERSION ${VERSION_MAJOR}.${VERSION_MINOR}.${VERSION_REVISION}${VERSION_SUFFI
set(SDCARD_REVISION "0018")
set(SDCARD_VERSION ${VERSION_MAJOR}.${VERSION_MINOR}V${SDCARD_REVISION})

cmake_minimum_required(VERSION 3.13)
cmake_policy(SET CMP0020 NEW)
cmake_policy(SET CMP0023 OLD)
if(POLICY CMP0042)
Expand Down Expand Up @@ -162,11 +163,6 @@ if(Qt5Core_FOUND OR FOX_FOUND)
endif()
endif(NOT DISABLE_COMPANION)

# Check for a file that is typically left from a OpenTX 2.1 build and abort if found
if (EXISTS ${RADIO_SRC_DIRECTORY}/stamp.h OR EXISTS ${RADIO_SRC_DIRECTORY}/translations/en.h)
message(FATAL_ERROR "Source directory contains files leftover from a OpenTX 2.1 build. Please run `git clean -f` in source directory (Careful: Will remove any extra files) or do a new clean git checkout")
endif()

# Windows-specific includes and libs shared by sub-projects
if(WIN32)
list(APPEND WIN_INCLUDE_DIRS "${RADIO_SRC_DIRECTORY}/thirdparty/windows/dirent")
Expand Down
4 changes: 2 additions & 2 deletions radio/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set(GUI_LANGUAGES CZ DE EN ES FR IT PT SK SE PL HU NL)
#set(TTS_LANGUAGES CZ DE EN ES FR IT PT SK SE PL HU NL RU)
set(TTS_LANGUAGES EN) # i6X dfplayer

set(PCB "X9D+" CACHE STRING "Radio type, one of: ${PCB_TYPES}")
set(PCB "I6X" CACHE STRING "Radio type, one of: ${PCB_TYPES}")
set_property(CACHE PCB PROPERTY STRINGS ${PCB_TYPES})
set(TRANSLATIONS "EN" CACHE STRING "Radio language, one of: ${GUI_LANGUAGES}")
set_property(CACHE TRANSLATIONS PROPERTY STRINGS ${GUI_LANGUAGES})
Expand All @@ -18,7 +18,7 @@ set(DEFAULT_MODE "" CACHE STRING "Default sticks mode")
set(FONT "STD" CACHE STRING "Choose font : STD or SQT5")
set_property(CACHE FONT PROPERTY STRINGS SQT5)

option(HELI "Heli menu" ON)
option(HELI "Heli menu" OFF)
dim13 marked this conversation as resolved.
Show resolved Hide resolved
option(FLIGHT_MODES "Flight Modes" ON)
option(CURVES "Curves" ON)
option(GVARS "Global variables" ON)
Expand Down
2 changes: 2 additions & 0 deletions radio/src/targets/common/arm/stm32/usb_conf.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@

/* EP1, TX buffer base address */
#define MSC_IN_TX_ADDRESS (0x98)
#define HID_IN_TX_ADDRESS (0x98)

/* EP2, Rx buffer base address */
#define MSC_OUT_RX_ADDRESS (0xD8)
#define HID_OUT_RX_ADDRESS (0xD8)

/* EP2 Tx buffer base address */
#define BULK_IN_TX_ADDRESS (0xC0)
Expand Down
63 changes: 22 additions & 41 deletions radio/src/targets/common/arm/stm32/usb_driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ extern "C" {
#endif

#include "opentx.h"
#include "board.h"
#include "debug.h"

static bool usbDriverStarted = false;
Expand Down Expand Up @@ -162,6 +163,8 @@ bool usbStarted()
}

#if !defined(BOOT)

#define PACKET_SIZE 8
/*
Prepare and send new USB data packet

Expand All @@ -171,60 +174,38 @@ bool usbStarted()
*/
void usbJoystickUpdate()
{
static uint8_t HID_Buffer[HID_IN_PACKET];
static uint8_t HID_Buffer[PACKET_SIZE];

// test to see if TX buffer is free
#if defined(STM32F0)
if (USBD_HID_SendReport(&USB_Device_dev, 0, 0) == USBD_OK) {
#else
if (USBD_HID_SendReport(&USB_OTG_dev, 0, 0) == USBD_OK) {
#endif
//buttons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here - please keep it as in EdgeTX/OpenTX, it keeps this project as close to EdgeTX as possible to backport features and track issues, see:
https://github.com/EdgeTX/edgetx/blob/main/radio/src/targets/common/arm/stm32/usb_driver.cpp

Copy link
Author

@dim13 dim13 Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue, that upstream implementation is plain broken and not worth to keep. Even with detection fix it does not work as generally expected. For example it depends on currently chosen model. You get different results with different models.

I would say, that it is would be more expected for it to "just work" as game controller independent from transmitter settings.

However I'm willing to keep old broken implementation in-place and make it configurable instead.

However keep in mind, that buttons are not working at all with old definition.

Copy link
Member

@ajjjjjjjj ajjjjjjjj Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buttons are not working, or are not set up in model inputs?

HID_Buffer[0] = 0;
HID_Buffer[1] = 0;
HID_Buffer[2] = 0;
for (int i = 0; i < 8; ++i) {
if ( channelOutputs[i+8] > 0 ) {
HID_Buffer[0] |= (1 << i);
}
if ( MAX_OUTPUT_CHANNELS>=24 && channelOutputs[i+16] > 0 ) {
HID_Buffer[1] |= (1 << i);
}
if ( MAX_OUTPUT_CHANNELS>=32 && channelOutputs[i+24] > 0 ) {
HID_Buffer[2] |= (1 << i);
}
}

//analog values
//uint8_t * p = HID_Buffer + 1;
for (int i = 0; i < 8; ++i) {
// 4 axes
HID_Buffer[0] = uint8_t(adcValues[STICK1] >> 4) - 0x7f;
HID_Buffer[1] = uint8_t(adcValues[STICK2] >> 4) - 0x7f;
HID_Buffer[2] = uint8_t(adcValues[STICK3] >> 4) - 0x7f;
HID_Buffer[3] = uint8_t(adcValues[STICK4] >> 4) - 0x7f;

int16_t value = channelOutputs[i] + 1024;
if ( value > 2047 ) value = 2047;
else if ( value < 0 ) value = 0;
#if defined(PCBI6X)
HID_Buffer[i*2 +2] = static_cast<uint8_t>(value & 0xFF);
HID_Buffer[i*2 +3] = static_cast<uint8_t>((value >> 8) & 0x07);
#else
HID_Buffer[i*2 +3] = static_cast<uint8_t>(value & 0xFF);
HID_Buffer[i*2 +4] = static_cast<uint8_t>((value >> 8) & 0x07);
#endif
// 2 pots
HID_Buffer[4] = uint8_t(adcValues[POT1] >> 4) - 0x7f;
HID_Buffer[5] = uint8_t(adcValues[POT2] >> 4) - 0x7f;

}

#if defined(PCBI6X)
// HID_Buffer index 8 & 9 causes mess. Looks like clock issue but cannot confirm.
// i reduced buttons to 16 so it will affect only one analog and remapped it [3] -> [5]
HID_Buffer[12] = HID_Buffer[8]; // ch[3] remap to ch[5] // channel 5 void
HID_Buffer[13] = HID_Buffer[9];
HID_Buffer[8] = 0;
HID_Buffer[9] = 0;
#endif
// 4 switches
// up: 10
// mid: 00
// dn: 01
HID_Buffer[6] = (~(uint8_t(adcValues[SW_A] >> 10) - 2) & 0x03)
| (~(uint8_t(adcValues[SW_B] >> 10) - 2) & 0x03) << 2
| (~(uint8_t(adcValues[SW_C] >> 10) - 2) & 0x03) << 4
| (~(uint8_t(adcValues[SW_D] >> 10) - 2) & 0x03) << 6;

#if defined(STM32F0)
USBD_HID_SendReport(&USB_Device_dev, HID_Buffer, HID_IN_PACKET);
USBD_HID_SendReport(&USB_Device_dev, HID_Buffer, PACKET_SIZE);
#else
USBD_HID_SendReport(&USB_OTG_dev, HID_Buffer, HID_IN_PACKET);
USBD_HID_SendReport(&USB_OTG_dev, HID_Buffer, PACKET_SIZE);
#endif
}
}
Expand Down
120 changes: 23 additions & 97 deletions radio/src/targets/common/arm/stm32/usbd_hid_joystick.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ static uint8_t USBD_HID_Setup (void *pdev,
USB_SETUP_REQ *req);

static const uint8_t *USBD_HID_GetCfgDesc (uint8_t speed, uint16_t *length);

static uint8_t USBD_HID_DataIn (void *pdev, uint8_t epnum);
/**
* @}
*/
Expand Down Expand Up @@ -112,47 +110,29 @@ static uint8_t USBD_HID_DataIn (void *pdev, uint8_t epnum);
__ALIGN_BEGIN static const uint8_t HID_JOYSTICK_ReportDesc[] __ALIGN_END =
{
0x05, 0x01, // USAGE_PAGE (Generic Desktop)
0x09, 0x05, // USAGE (Game Pad)
0x09, 0x04, // USAGE (Joystick)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is 0x05 in EdgeTX, I'm not against the change but is there a specific reason to change it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Value from original firmware, IMHO more suitable for the use. Doesn't do much however.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is port of OpenTX/EdgeTX and there is no specific reason to change that, it should stay as it was.

0xa1, 0x01, // COLLECTION (Application)
0xa1, 0x00, // COLLECTION (Physical)
0x05, 0x09, // USAGE_PAGE (Button)
0x19, 0x01, // USAGE_MINIMUM (Button 1)
#if defined(PCBI6X)
0x29, 0x10, // USAGE_MAXIMUM (Button 16)
#else
0x29, 0x18, // USAGE_MAXIMUM (Button 24)
#endif
0x15, 0x00, // LOGICAL_MINIMUM (0)
0x25, 0x01, // LOGICAL_MAXIMUM (1)
#if defined(PCBI6X)
0x95, 0x10, // REPORT_COUNT (16)
#else
0x95, 0x18, // REPORT_COUNT (24)
#endif
0x75, 0x01, // REPORT_SIZE (1)
0x81, 0x02, // INPUT (Data,Var,Abs)
0x05, 0x01, // USAGE_PAGE (Generic Desktop)
0x09, 0x30, // USAGE (X)
0x09, 0x31, // USAGE (Y)
0x09, 0x32, // USAGE (Z)
#if defined(PCBI6X)
0x09, 0x35, // USAGE (Rz)
0x09, 0x33, // USAGE (Rx)
0x09, 0x34, // USAGE (Ry)
#else
0x09, 0x33, // USAGE (Rx)
0x09, 0x34, // USAGE (Ry)
0x09, 0x35, // USAGE (Rz)
#endif
0x09, 0x36, // USAGE (Slider)
0x09, 0x36, // USAGE (Slider)
0x16, 0x00, 0x00, // LOGICAL_MINIMUM (0)
0x26, 0xFF, 0x07, // LOGICAL_MAXIMUM (2047)
0x75, 0x10, // REPORT_SIZE (16)
0x15, 0x81, // LOGICAL_MINIMUM (-127)
0x25, 0x7F, // LOGICAL_MAXIMUM (127)
0x75, 0x08, // REPORT_SIZE (8)
0x95, 0x06, // REPORT_COUNT (6)
0x81, 0x02, // INPUT (Data,Var,Abs)
0x05, 0x09, // USAGE_PAGE (Button)
0x19, 0x01, // USAGE_MINIMUM (Button 1)
0x29, 0x08, // USAGE_MAXIMUM (Button 8)
0x15, 0x00, // LOGICAL_MINIMUM (0)
0x25, 0x01, // LOGICAL_MAXIMUM (1)
0x75, 0x01, // REPORT_SIZE (1)
0x95, 0x08, // REPORT_COUNT (8)
0x81, 0x02, // INPUT (Data,Var,Abs)
0xc0, // END_COLLECTION
0xc0 // END_COLLECTION
0xc0, // END_COLLECTION
};


Expand All @@ -168,7 +148,7 @@ const USBD_Class_cb_TypeDef USBD_HID_cb =
USBD_HID_Setup,
NULL, /*EP0_TxSent*/
NULL, /*EP0_RxReady*/ /* STATUS STAGE IN */
USBD_HID_DataIn, /*DataIn*/
NULL, /*DataIn*/
NULL, /*DataOut*/
NULL, /*SOF */
USBD_HID_GetCfgDesc,
Expand All @@ -179,7 +159,7 @@ const USBD_Class_cb_TypeDef USBD_HID_cb =
USBD_HID_Setup,
NULL, /*EP0_TxSent*/
NULL, /*EP0_RxReady*/
USBD_HID_DataIn, /*DataIn*/
NULL, /*DataIn*/
NULL, /*DataOut*/
NULL, /*SOF */
NULL,
Expand Down Expand Up @@ -229,7 +209,7 @@ __ALIGN_BEGIN static const uint8_t USBD_HID_CfgDesc[USB_HID_CONFIG_DESC_SIZ] __A
0x01, /*bConfigurationValue: Configuration value*/
0x00, /*iConfiguration: Index of string descriptor describing
the configuration*/
0xE0, /*bmAttributes: bus powered and Support Remote Wake-up */
0xC0, /*bmAttributes: bus powered */
0x32, /*MaxPower 100 mA: this current is used for detecting Vbus*/

/************** Descriptor of Joystick Mouse interface ****************/
Expand Down Expand Up @@ -261,16 +241,12 @@ __ALIGN_BEGIN static const uint8_t USBD_HID_CfgDesc[USB_HID_CONFIG_DESC_SIZ] __A

HID_IN_EP, /*bEndpointAddress: Endpoint Address (IN)*/
0x03, /*bmAttributes: Interrupt endpoint*/
HID_IN_PACKET, /*wMaxPacketSize: 4 Byte max */
0x40, /*wMaxPacketSize: 4 Byte max */
0x00,
0x02, /*bInterval: Polling Interval (2 ms)*/
0x07, /*bInterval: Polling Interval (7 ms)*/
ajjjjjjjj marked this conversation as resolved.
Show resolved Hide resolved
/* 34 */
} ;



static uint8_t ReportSent;

/**
* @}
*/
Expand All @@ -289,6 +265,7 @@ static uint8_t ReportSent;
static uint8_t USBD_HID_Init (void *pdev,
uint8_t cfgidx)
{
DCD_PMA_Config(pdev, HID_IN_EP,USB_SNG_BUF, HID_IN_TX_ADDRESS);

/* Open EP IN */
DCD_EP_Open(pdev,
Expand All @@ -299,18 +276,7 @@ static uint8_t USBD_HID_Init (void *pdev,
#else
USB_OTG_EP_INT);
#endif

/* Open EP OUT */
DCD_EP_Open(pdev,
HID_OUT_EP,
HID_OUT_PACKET,
#if defined(STM32F0)
USB_EP_INT);
#else
USB_OTG_EP_INT);
#endif

ReportSent = 1;
return USBD_OK;
}

Expand All @@ -326,9 +292,7 @@ static uint8_t USBD_HID_DeInit (void *pdev,
{
/* Close HID EPs */
DCD_EP_Close (pdev , HID_IN_EP);
DCD_EP_Close (pdev , HID_OUT_EP);

ReportSent = 1;
return USBD_OK;
}

Expand Down Expand Up @@ -430,29 +394,16 @@ static uint8_t USBD_HID_Setup (void *pdev,
uint8_t USBD_HID_SendReport(USB_CORE_HANDLE *pdev, uint8_t * report, uint16_t len)
{
if (pdev->dev.device_status == USB_CONFIGURED) {
if (ReportSent) {
if (report) {
ReportSent = 0;
DCD_EP_Tx (pdev, HID_IN_EP, report, len);
}
return USBD_OK;
}
DCD_EP_Tx (pdev, HID_IN_EP, report, len);
}
return USBD_FAIL;
return USBD_OK;
}
#else
uint8_t USBD_HID_SendReport(USB_OTG_CORE_HANDLE *pdev, uint8_t * report, uint16_t len)
{
if (pdev->dev.device_status == USB_OTG_CONFIGURED) {
if (ReportSent) {
if (report) {
ReportSent = 0;
DCD_EP_Tx (pdev, HID_IN_EP, report, len);
}
return USBD_OK;
}
}
return USBD_FAIL;
DCD_EP_Tx (pdev, HID_IN_EP, report, len);
return USBD_OK;
}
#endif

Expand All @@ -469,31 +420,6 @@ static const uint8_t *USBD_HID_GetCfgDesc (uint8_t speed, uint16_t *length)
return USBD_HID_CfgDesc;
}

/**
* @brief USBD_HID_DataIn
* handle data IN Stage
* @param pdev: device instance
* @param epnum: endpoint index
* @retval status

This function is called when buffer has been sent over the USB.
The TX buffer is now empty and can be filled with new data.
*/
static uint8_t USBD_HID_DataIn (void *pdev,
uint8_t epnum)
{
ReportSent = 1;
#if defined(STM32F0)
// ((USBD_HID_HandleTypeDef *)pdev->pClassData)->state = HID_IDLE;
// if (epnum == 1) PrevXferDone = 1;
#else
/* Ensure that the FIFO is empty before a new transfer, this condition could
be caused by a new transfer before the end of the previous transfer */
DCD_EP_Flush(pdev, HID_IN_EP);
#endif
return USBD_OK;
}

/**
* @}
*/
Expand Down
4 changes: 2 additions & 2 deletions radio/src/targets/flysky/board.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,12 +335,12 @@ enum Analogs {
STICK4,
SW_A,
SW_B,
SW_C,
SW_D,
POT_FIRST,
POT1 = POT_FIRST,
POT2,
POT_LAST = POT2,
SW_C,
ajjjjjjjj marked this conversation as resolved.
Show resolved Hide resolved
SW_D,
TX_VOLTAGE,
NUM_ANALOGS
};
Expand Down
2 changes: 2 additions & 0 deletions tools/flash.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/bash
dfu-util -a0 -s 0x08000000:leave -d 0483:df11 -D firmware.bin