Repository navigation
Conversation
… the buses i2c_speed set one clock for every bus, so a bus with a chip limited to 400 kHz held back a barometer on another bus that could run at 800 kHz. It is replaced by i2c1_speed..i2c4_speed (new PG, only for the buses a target enables; i2c1_speed also sets a software I2C bus). The field leaves systemConfig, whose PG version goes up: that group returns to its defaults on upgrade. 1000KHZ joins the speeds, for a bus whose chips all support Fast-mode Plus. MSP2_INAV_I2C_BUSES (0x2238) reports each started bus with the clock it runs at and the detected accelerometer, barometer, compass, rangefinder and pitot sensors on it (bus, address, role, *_hardware value), so a configurator can show what shares a bus before raising its speed.
Contributor
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
The Configurator shows each bus's i2cN_speed, and it reboots the board when it saves, so the clock the message reported only repeated it. The field goes, and with it i2cGetSpeed(), which had no other caller.
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #12148 249 targets built. Find your board's
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
i2c_speedsets one clock for every I2C bus. On a board with the compass on one bus and the barometer on another, the slower chip decides for both: an IST8310 is rated for 400 kHz, so the barometer can't run at 800 kHz either, though the Configurator suggests it. This replacesi2c_speedwith one setting per bus, and adds an MSP message that lists the buses with the sensors on each, so the Configurator can show what shares a bus before a speed is raised (iNavFlight/inav-configurator#2835).It targets maintenance-11.x because it removes a setting and changes the layout of a parameter group.
What changes
i2c_speedis gone.i2c1_speedtoi2c4_speedtake its place, each only on targets that enable that bus (USE_I2C_DEVICE_n), with the same values plus1000KHZand 400KHZ as default. They're stored in a new parameter group,PG_I2C_BUS_CONFIG(1054; 1048 to 1053 are claimed by open PRs). On a board with software I2C (MATEKF405OSD),i2c1_speedsets that bus.1000KHZ(Fast-mode Plus) is new. F7, H7 and AT32 compute its timing with the Fm+ figures that 800KHZ already uses, RP2350 sets the baud rate, and F4 runs past its I2C rating there, as it already does at 800KHZ. I don't set the Fm+ drive bits: they only matter with pull-ups stronger than the usual 2.2 to 4.7 kΩ. With slow edges the I2C peripheral waits for the clock line to rise, so a bus with weak pull-ups or long wires runs slower than 1 MHz, or fails.i2cSetSpeed()now takes the bus, andi2cIsInitialised()is new, in every I2C driver (HAL, F4, AT32, RP2350 and software).MSP2_INAV_I2C_BUSES(0x2238, out): every bus the board started, then every detected accelerometer, barometer, compass, rangefinder and pitot sensor on I2C, with its bus, address, role (sensorIndex_e) and hardware (its*_hardwarevalue). A bus that shares its pins with a UART in use isn't started, so it's left out. A software bus is reported as I2C1 with bit 7 of the bus number set. It's documented inmsp_messages.json(patch version up).Upgrade
Removing
i2c_speedfromsystemConfig_tchanges its layout, soPG_SYSTEM_CONFIGgoes from version 11 to 12 (7 to 8 on targets withoutUSE_AUTO_TRANSITION). That group returns to its defaults on the first boot after the upgrade:debug_modeandthrottle_tilt_comp_str;vtol_*);nameandpilot_name.Pasting a
diff alltaken before the upgrade restores them. Aset i2c_speed = ...line in that diff now gives an error, and every bus starts at 400KHZ until itsi2cN_speedis set.Size
Against maintenance-11.x:
Most of it is the MSP message. Nothing new runs from the scheduler, and the KAKUTEF7 and KAKUTEF7HDV ITCM doesn't change.
Testing
Builds with
WARNINGS_AS_ERRORS: TBS_LUCID_H7_WING, MATEKF722, MATEKF405, MATEKF405OSD (software I2C), MATEKF411, OMNIBUSF4 (I2C2 shares UART3), KAKUTEF7, KAKUTEF7HDV, KAKUTEH7WING (I2C4), BLUEBERRYF435WING (AT32) and RP2350_PICO. All 559 unit tests pass.TBS Lucid H7 Wing, with an IST8310 compass on I2C1 (through the GPS cable) and the DPS310 on I2C2. The message listed both buses and both sensors at the right addresses. The compass task time followed the I2C1 clock while I2C2 was set to something else:
i2c1_speedThe IST8310 is rated for 400 kHz: the 800 and 1000 kHz rows only show that the clock changes, they're not a recommendation. With I2C2 at 1000KHZ the DPS310 kept working and the I2C error count didn't move.
Not tested
i2c1_speed, and the "I2C1 (software)" row in the Configurator.Overlaps
msp_protocol_v2_inav.h,msp_messages.json,README.md): additions next to each other, where both sides stay. When other parameter groups come in,PG_INAV_ENDhas to stay at the highest id (1054), orMSP2_COMMON_PG_LISTleaves this group out.