search

betaflight / config

Betaflight target definitions
GNU General Public License v3.0
24 stars 85 forks source link

comment out unsupported defines, and perhaps fix some #420

Open ctzsnooze opened 1 month ago

ctzsnooze commented 1 month ago

While testing this grep string tp find the defines we use,

grep --exclude-dir=config -Irh '^[^/].*' src/ | grep -wo 'USE_[0-9A-Z_]\+' | sort | uniq

If we remove the exclusion on the config directory, a whole lot of config files can be seen to reference defines which do not exist. Some are being changed to existing defines that should work, others are commented out pending a formal resolution of what should be done with them.

Changed defines: From To Boards
USE_I2C1_PULLUP ON I2C1_PULLUP ON Alienflight
USE_I2C3_PULLUP ON I2C3_PULLUP ON RacePit
USE_FLASH_W25Q128 USE_FLASH_W25Q128FV SPRacingH7EF @hydra ??
USE_FLASH_W25P16 USE_FLASH_M25P16 BluejayF4, CycloneF722
USE_FLASH_W25Q256 USE_FLASH_W25Q128FV BrahmaF405
Commented out defines: Define Reason Boards
USE_RX_EXPRESSLRS_TELEMETRY no such define lots of boards @phobos21 ??
USE_ACCGYRO_ASM330LHH no such define NeutronRCF435SE
USE_FAKE_ACC no such define NucleoF446
USE_FAKE_GYRO no such define NucleoF446
USE_FAKE_MAG no such define NucleoF446
USE_SONAR no such define NucleoF446
USE_CC2500 no such define NucleoF446
USE_MPU_DATA_READY_SIGNAL no such define SPRacingF7EVO @hydra ??
USE_GYRO_FAST_KALMAN no such define YupiF4

Please carefully check that these are sensible changes :-)

nerdCopter commented 1 month ago 
* #define `USE_MPU_DATA_READY_SIGNAL` should probably be `USE_MAG_DATA_READY_SIGNAL`

USE_MPU_DATA_READY_SIGNAL used to be a legitimate define, at least in 3.5.x. It's probably not _MAG_. Maybe @hydra can clarify. (ref ./configs/SPRACINGH7EVO/config.h:116)

ctzsnooze commented 1 month ago

@haslinghuis @nerdCopter thanks for the advice. For now I've made some updates and summarised them in the tables above.

ctzsnooze commented 1 month ago

Here are a few defines for which I can't figure out where they get 'defined'... so long as that's OK, I'm good with it:

Define Code Concern
USE_ADC3_DIRECT_HAL_INIT stm32h7xx.c used but not set
USE_ADC_INTERRUPT adc_stm32g4xx.c, adc_stm32h7xx.c used but not set
USE_BIND_ADDRESS_FOR_DATA_STATE nrf24_inav.c used but define line is commented out
USE_BRUSHED msp_build_info.c sets a build option, but never defined
USE_BST tasks.c, scheduler.h used to include i2c_bst.h but never defined
USE_CCM_CODE system.c, common_post.h never defined
USE_HAL_xxx_xxx stm32h7xx_hal_conf.h local to that file, not really user defines
haslinghuis commented 1 month ago

@ctzsnooze agree #define USE_RX_EXPRESSLRS_TELEMETRY can be removed.

CapnBry commented 1 month ago

Hey Betaflight crew, I'm out of town, don't have my glasses, and haven't looked at the ELRS SPI code in nearly a year, but I'd be happy to take a close look at comment Sunday night/ Monday morning.

CapnBry commented 1 month ago

I can also confirm USE_RX_EXPRESSLRS_TELEMETRY has never been used, not even in the original code for ExpressLRS 2.x SPI, so it should be removed.

hydra commented 1 month ago

IIRC USE_MPU_DATA_READY_SIGNAL was used so that FC's would fail gyro checks on boot if the INT pin wasn't soldered on correctly, prior to support for EXTI was added, wasn't enabled by default as some old FC's didn't have the gyro INT signal connected to the MCU.

Not sure what's happened to this functionality but if it's been removed it should be restored so that users and factories can be sure their gyros are working correctly.

EDIT:

ah, I was thinking of ENSURE_MPU_DATA_READY_IS_LOW.

that probably should be cleaned up somewhat so that that it's always used, and should be based on the appropriate INT signal level for the gyro being used, after it's correctly detected.

MPU int signal level failure should result in some form of error or status that can be retieved.

Perhaps this ^^^ should be moved into a separate issue so it can be properly tracked and planned?

Also, ENSURE_MPU_DATA_READY_IS_LOW could probably be tied to the gating for all the gyros like ICM20602 and later, as IIRC at that point all the FCs should have been using the INT signal. And if they don't, perhaps plan to make them unsupported for the next release.

Not sure if the hardware guidelines have a note regarding the requirement for the gyro int signal, but if not it should probably be added and pin selection for gyro int signals should either be on a gpio pin with a dedicated interrupt, e.g. EXTI1/EXTI2/EXTI3/EXTI/4, and not EXTI9_5 or EXTI15_10 which should only be used for lower frequency interrupts due to the additional overhead in determining which exti signal caused the interrupt.

hydra commented 1 month ago

USE_FLASH_W25Q128 USE_FLASH_W25Q128FV

Yes, this is fine. IIRC this came about around the time I was working on QSPI support for the W25Q128 flash, but the EF doesn't have the octospi drivers enabled as the octospi usage is invisible to BF on this target because it boots and runs firmware from octospi but BF stores config on the 2nd flash chip which is connected via SPI. Making the change should be tested by a maintainer with access to an H7EF. Please confirm this has been done before merging this PR. IIRC @haslinghuis has one.

haslinghuis commented 1 month ago

@hydra this is without the change:

image

# Betaflight / STM32H730 (SP7E) 4.6.0 May 13 2024 / 14:27:18 (2d3c8eea3) MSP API: 1.46
# config rev: 45995c5
# board: manufacturer_id: SPRO, board_name: SPRACINGH7EF

MCU H730 Clock=520MHz, Vref=3.34V, Core temp=55degC
Stack size: 2048, Stack address: 0x20020000
Configuration: CONFIGURED, size: 3839, max available: 65536
Devices detected: SPI:2, I2C:0
Gyros detected: gyro 1, gyro 2 locked dma
GYRO=ICM42688P, ACC=ICM42688P
OSD: MSP (30 x 13)
BUILD KEY: c39e18bc95a96534881c96c161244871 (4.6.0-dev)
System Uptime: 29 seconds, Current Time: 2024-05-13T14:29:11.795+00:00
CPU:37%, cycle time: 121, GYRO rate: 8264, RX rate: 15, System rate: 9
Voltage: 301 * 0.01V (1S battery - CRITICAL)
I2C Errors: 10
FLASH: JEDEC ID=0x00ef4015 2M
GPS: NOT ENABLED
Arming disable flags: RXLOSS ANGLE CLI MSP
hydra commented 1 month ago

Define Code Concern USE_BRUSHED msp_build_info.c sets a build option, but never defined

probably dates back to the CJMCU brushed board from Cleanflight.

Define Code Concern USE_BST tasks.c, scheduler.h used to include i2c_bst.h but never defined

probably dates back to the TBS PowerCube stack and related targets.

Define Code Concern USE_CCM_CODE system.c, common_post.h never defined

probably no-longer exists due to removal of F3 support which used Core Coupled Memory (CCM).

image

hydra commented 1 month ago

@hydra this is without the change:

FLASH: JEDEC ID=0x00ef4015 2M

yes, this is what you're looking for after you make the change, likely though it just wouldn't boot due to having no-where to store the config if the change didn't work.

The board you have has a W25Q16JV fitted:

image

haslinghuis commented 1 month ago

@hydra

yes, this is what you're looking for after you make the change, likely though it just wouldn't boot due to having no-where to store the config if the change didn't work.

This is before the change. So it seems W25Q128FV define is not needed?

hydra commented 1 month ago

@hydra

yes, this is what you're looking for after you make the change, likely though it just wouldn't boot due to having no-where to store the config if the change didn't work.

This is before the change. So it seems W25Q128FV define is not needed?

seems not then; due to the octospi reasons I mentioned before, in which case it can be removed from the target. As and when I have time to work on octospi and spi I can make corresponding target updates so that BF stores it's config on the memory mapped flash so that the entire SPI flash is used for logging.