qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.16k stars 39.1k forks source link

Compile error with Chibi OS and USART serial. #19617

Closed xisang233 closed 1 year ago

xisang233 commented 1 year ago

Issue Description

I tried to make a split keyboard on stm32f103c8t6 and I set serial driver options in config.h. Like https://docs.qmk.fm/#/serial_driver. This is my code:

#define SERIAL_USART_TX_PIN A9
#define SERIAL_USART_DRIVER SD1
#define SERIAL_USART_FULL_DUPLEX
#define SERIAL_USART_RX_PIN A10

When I compile it, there was an error like:

Compiling: platforms/chibios/drivers/serial_usart.c
                      In file included from platforms/chibios/drivers/serial_usart.c:5:
platforms/chibios/drivers/serial_usart.h:11: error: "SERIAL_USART_TX_PIN" redefined [-Werror]
   11 | #    define SERIAL_USART_TX_PIN SOFT_SERIAL_PIN
      |
In file included from <command-line>:
./keyboards/xisang/stm_test/config.h:35: note: this is the location of the previous definition
   35 | #define SERIAL_USART_TX_PIN A9 //TX?

When I tried to delect the TX_PIN and RX_PIN options like this, the compile works(now I havn't a ST Link so I couldn't flash and test it).

#define SERIAL_USART_DRIVER SD1
#define SERIAL_USART_FULL_DUPLEX

I'm not very familiar with stm32 and Chibi OS, but I think A9 and A10 was selected as TX_PIN and RX_PIN because I used #define SERIAL_USART_DRIVER SD1. So the #define SERIAL_USART_TX_PIN A9 is incorrect here. Perhaps it is new feature in Chibi OS and the manual document are outdated. So the manual document should be corrected.

sigprof commented 1 year ago

You probably have either #define SOFT_SERIAL_PIN something in config.h, or split.soft_serial_pin in info.json (which also results in defining SOFT_SERIAL_PIN at the C level in the generated header). The USART serial code then tries to define SERIAL_USART_TX_PIN to the same value as SOFT_SERIAL_PIN, presumably so that you could switch from soft serial to USART half-duplex serial without changing anything except SERIAL_DRIVER.

The default values of SERIAL_USART_TX_PIN and SERIAL_USART_RX_PIN are indeed A9 and A10, but they are not adjusted automatically according to the SERIAL_USART_DRIVER setting, so it is possible to specify an incorrect combination of those settings without getting any error messages from the compiler.

xisang233 commented 1 year ago

You probably have either #define SOFT_SERIAL_PIN something in config.h, or split.soft_serial_pin in info.json (which also results in defining SOFT_SERIAL_PIN at the C level in the generated header). The USART serial code then tries to define SERIAL_USART_TX_PIN to the same value as SOFT_SERIAL_PIN, presumably so that you could switch from soft serial to USART half-duplex serial without changing anything except SERIAL_DRIVER.

The default values of SERIAL_USART_TX_PIN and SERIAL_USART_RX_PIN are indeed A9 and A10, but they are not adjusted automatically according to the SERIAL_USART_DRIVER setting, so it is possible to specify an incorrect combination of those settings without getting any error messages from the compiler.

Thank you. I didn't set anything about SOFT_SERIAL_PIN. But these code were changed from code with AVR chips(it worked on Pro Micro) and soft serial so perhaps I forgot somewere to delete completely. I will try to flash my code to STM32 chips and try if it works.

drashna commented 1 year ago

Are you using a Pro Micro RP2040 (or the board file for that)?

xisang233 commented 1 year ago

Are you using a Pro Micro RP2040 (or the board file for that)?

Hello. I'm using STM32F103C6T6(blue pill) and I tried many ways to let them connect to each other. But they dosn't work.

Perhaps something was wrong because I didn't use C8T6 but C6T6. I will buy a new STM32F103C8T6 board and try it again.

fauxpark commented 1 year ago

Might be related? https://github.com/qmk/qmk_firmware/pull/19075 As I noted I tested the UART driver with and without the above PR and couldn't get it to work on F103. It may be an issue with the serial driver in ChibiOS itself.

xisang233 commented 1 year ago

Might be related? #19075 As I noted I tested the UART driver with and without the above PR and couldn't get it to work on F103. It may be an issue with the serial driver in ChibiOS itself.

Wonderful! I changed \platforms\chibios\drivers and it WORKS.

My code in config.h was like:

#define SERIAL_USART_FULL_DUPLEX     // Enable full duplex operation mode.
#define SERIAL_USART_DRIVER SD1

It works without #define SERIAL_USART_TX_PIN setting. And the compile was still failed when I set #define SERIAL_USART_TX_PIN A9. So I think the manual document should be changed.

xisang233 commented 1 year ago

By the way, I also tried bitbang and SIO. But they didn't work.

infinityis commented 1 year ago

To be clear, the reason sigprof suggests that SOFT_SERIAL_PIN must be defined somewhere already is because of the lines surrounding line 11 (where the error occurred) in serial_usart.h, specifically:

10   #if defined(SOFT_SERIAL_PIN)
11   #    define SERIAL_USART_TX_PIN SOFT_SERIAL_PIN
12   #endif

You literally can't get an error on line 11 unless SOFT_SERIAL_PIN is defined somewhere. If SOFT_SERIAL_PIN is defined somewhere, line 11 will be processed and the error you are seeing will occur (because you are manually defining SERIAL_USART_TX_PIN and line 11 is trying to do the same thing). If SOFT_SERIAL_PIN is not defined anywhere, then line 11 never gets executed at all, and the compiler (or rather precompiler) can't generate an error there.

Put another way: the error you are getting means that line 11 of serial_usart.h is being executed, and line 11 can only be executed if SOFT_SERIAL_PIN is defined somewhere in the code (or assigned in a json file).

With SOFT_SERIAL_PIN being defined somewhere else, the error is correct in pointing out that SERIAL_USART_TX_PIN shouldn't be assigned. Either SOFT_SERIAL_PIN or SERIAL_USART_TX_PIN should be assigned a value, but not both, because they both ultimately do the same thing. Without reviewing your code, I can't tell where the soft serial pin might be getting set; I can only say that it is being set somewhere based on the error you are seeing. If you push a commit to your fork on Github, that may help us see where it may be getting inadvertently assigned.

What tzarc mentions in the other PR is also correct: any changes to the uart driver do not affect the serial_usart driver that you are using. While it is true that within chibios these two functionalities make use of the same underlying hardware, the #defines in QMK are completely independent for the two drivers as they have different purposes. So a change to uart.h and uart.c should have no effect on your ability to get serial_usart working.

Lastly, you've said you changed \platforms\chibios\drivers and that it works, but (1) we don't know how or what you changed there in there to make it work, and (2) unless you found and fixed a bug in the driver code, there generally shouldn't need to be any such changes made to core QMK code needed to get a keyboard to work. If you add a keyboard to QMK through a pull request, and that pull request also incorporates changes to core code (like in the \platforms\chibios\drivers folder), you will be asked to separate out the two changes, since core code changes are handled separately from keyboard-specific changes additions.

If you can share the code you are using, that will go a long way towards helping us understand what is and is not working, and where changes should be made to get your code running properly. It is always possible that there is a bug in QMK, and if your code reveals that SOFT_SERIAL_PIN is being incorrectly/automatically assigned somehow by the core code (as opposed to your keyboard-specific code), that would be a very good thing to fix.

xisang233 commented 1 year ago

Sorry everyone. I know what made my firmware don't work. I still using old QMK 0.18 and all of them happened. After I pulled new QMK 0.19, the problem solved. Code like:

#define SERIAL_USART_FULL_DUPLEX
#define SERIAL_USART_TX_PIN A9
#define SERIAL_USART_RX_PIN A10

can be compiled and works without any problem. That was my fault.