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 led strip for h7 #12890

Merged
merged 3 commits into from Jun 17, 2023
Merged

fix led strip for h7 #12890

merged 3 commits into from Jun 17, 2023

Conversation

freasy
Copy link
Member

@freasy freasy commented Jun 16, 2023

This fixes LED STRIP function for H7

@github-actions
Copy link

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #12890 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@KarateBrot KarateBrot added this to the 4.5 milestone Jun 16, 2023
src/main/target/common_pre.h Outdated Show resolved Hide resolved
Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

Logic needs update here also:

If we add USE_LEDSTRIP_CACHE_MGMT for H7 unconditional this code needs to be revised as the condition on line 62 will be unreachable.

#ifdef USE_LEDSTRIP_CACHE_MGMT
// WS2811_DMA_BUFFER_SIZE is multiples of uint32_t
// Number of bytes required for buffer
#define WS2811_DMA_BUF_BYTES (WS2811_DMA_BUFFER_SIZE * sizeof(uint32_t))
// Number of bytes required to cache align buffer
#define WS2811_DMA_BUF_CACHE_ALIGN_BYTES ((WS2811_DMA_BUF_BYTES + 0x20) & ~0x1f)
// Size of array to create a cache aligned buffer
#define WS2811_DMA_BUF_CACHE_ALIGN_LENGTH (WS2811_DMA_BUF_CACHE_ALIGN_BYTES / sizeof(uint32_t))
DMA_RW_AXI __attribute__((aligned(32))) uint32_t ledStripDMABuffer[WS2811_DMA_BUF_CACHE_ALIGN_LENGTH];
#else
#if defined(STM32F7)
FAST_DATA_ZERO_INIT uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#elif defined(STM32H7)
DMA_RAM uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#else
uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#endif
#endif

Copy link
Member

@SteveCEvans SteveCEvans left a comment

Choose a reason for hiding this comment

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

Good spot.

@SteveCEvans
Copy link
Member

Tested on MATEKH743 with 24 LEDs.

@freasy
Copy link
Member Author

freasy commented Jun 16, 2023

Logic needs update here also:

If we add USE_LEDSTRIP_CACHE_MGMT for H7 unconditional this code needs to be revised as the condition on line 62 will be unreachable.

#ifdef USE_LEDSTRIP_CACHE_MGMT
// WS2811_DMA_BUFFER_SIZE is multiples of uint32_t
// Number of bytes required for buffer
#define WS2811_DMA_BUF_BYTES (WS2811_DMA_BUFFER_SIZE * sizeof(uint32_t))
// Number of bytes required to cache align buffer
#define WS2811_DMA_BUF_CACHE_ALIGN_BYTES ((WS2811_DMA_BUF_BYTES + 0x20) & ~0x1f)
// Size of array to create a cache aligned buffer
#define WS2811_DMA_BUF_CACHE_ALIGN_LENGTH (WS2811_DMA_BUF_CACHE_ALIGN_BYTES / sizeof(uint32_t))
DMA_RW_AXI __attribute__((aligned(32))) uint32_t ledStripDMABuffer[WS2811_DMA_BUF_CACHE_ALIGN_LENGTH];
#else
#if defined(STM32F7)
FAST_DATA_ZERO_INIT uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#elif defined(STM32H7)
DMA_RAM uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#else
uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#endif
#endif

What do you mean?

@haslinghuis
Copy link
Member

Guess these line are no longer used:
image

@freasy
Copy link
Member Author

freasy commented Jun 16, 2023

Guess these line are no longer used:
image

That's true, i don't know if there is a way, this can be a error, when removed.

@haslinghuis
Copy link
Member

You could check the change - or leave a note / TODO for later

@freasy
Copy link
Member Author

freasy commented Jun 16, 2023

You could check the change - or leave a note / TODO for later

I will test it later and/or leave a Todo.

Good find 👍

@blckmn
Copy link
Member

blckmn commented Jun 16, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> FAIL
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> FAIL
  • assigned to an approver -> FAIL
  • approver count at least three -> PASS

@BMSThomas
Copy link

BMSThomas commented Jun 16, 2023

G'day, no idea if I'm using GitHub correctly but @ctzsnooze reccomended I go here so here I am! Just loaded this commit to try get LEDs going on my Foxeer H743 & still having some troubles, but its working much more then it was before (before was totally random, now there some consistency). If I set the colour red, I get no LED colour. If I set green, I get a pulse of random colours, a pulse of green & then nothing. If I set Green with larson scanner I get an RGB smooth colour changing effect! It's not optimal but its consistent at least.

Also thanks so much for working on this swiftly!! Need LEDs for MultiGP IO, so really really appreciate the work!!

@sugaarK
Copy link
Member

sugaarK commented Jun 16, 2023

I’ll run a bunch of combos tonight.
@BMSThomas are you using the latest nightly configurator? I think that’s also part of it

@freasy
Copy link
Member Author

freasy commented Jun 17, 2023

G'day, no idea if I'm using GitHub correctly but @ctzsnooze reccomended I go here so here I am! Just loaded this commit to try get LEDs going on my Foxeer H743 & still having some troubles, but its working much more then it was before (before was totally random, now there some consistency). If I set the colour red, I get no LED colour. If I set green, I get a pulse of random colours, a pulse of green & then nothing. If I set Green with larson scanner I get an RGB smooth colour changing effect! It's not optimal but its consistent at least.

Also thanks so much for working on this swiftly!! Need LEDs for MultiGP IO, so really really appreciate the work!!

Which configurator version are you using?
Please try with the latest nightly again.

Also, how many LEDs are installed? Maybe too much for just USB power? Try with a lipo as well please.

Copy link
Member

@sugaarK sugaarK left a comment

Choose a reason for hiding this comment

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

Test on 2 setups using foxeer h7 mini.. they work as they should if you have the right configurator.. is there any reason not to resolve the contention and merge this

@sugaarK
Copy link
Member

sugaarK commented Jun 17, 2023

Logic needs update here also:

If we add USE_LEDSTRIP_CACHE_MGMT for H7 unconditional this code needs to be revised as the condition on line 62 will be unreachable.

#ifdef USE_LEDSTRIP_CACHE_MGMT
// WS2811_DMA_BUFFER_SIZE is multiples of uint32_t
// Number of bytes required for buffer
#define WS2811_DMA_BUF_BYTES (WS2811_DMA_BUFFER_SIZE * sizeof(uint32_t))
// Number of bytes required to cache align buffer
#define WS2811_DMA_BUF_CACHE_ALIGN_BYTES ((WS2811_DMA_BUF_BYTES + 0x20) & ~0x1f)
// Size of array to create a cache aligned buffer
#define WS2811_DMA_BUF_CACHE_ALIGN_LENGTH (WS2811_DMA_BUF_CACHE_ALIGN_BYTES / sizeof(uint32_t))
DMA_RW_AXI __attribute__((aligned(32))) uint32_t ledStripDMABuffer[WS2811_DMA_BUF_CACHE_ALIGN_LENGTH];
#else
#if defined(STM32F7)
FAST_DATA_ZERO_INIT uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#elif defined(STM32H7)
DMA_RAM uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#else
uint32_t ledStripDMABuffer[WS2811_DMA_BUFFER_SIZE];
#endif
#endif

Has this been resolved? Or does it meed another PR ?

@haslinghuis haslinghuis merged commit ac84115 into betaflight:master Jun 17, 2023
20 checks passed
@BMSThomas
Copy link

Hey Guys!! Just tried Nightly build and its working perfectly now!! Thank you so much for getting this together on such short notice!! Going to finish configuring the rest of the fleet now! Cheers!!

@ctzsnooze
Copy link
Member

Thanks @freasy !!

@SteveCEvans SteveCEvans mentioned this pull request Jul 20, 2023
haslinghuis pushed a commit to haslinghuis/betaflight that referenced this pull request Jul 20, 2023
* fix led strip for h7

* account for cloud build custom defines

* remove unused code
haslinghuis added a commit that referenced this pull request Aug 6, 2023
* fix led strip for h7 (#12890)

* fix led strip for h7

* account for cloud build custom defines

* remove unused code

* Fix per review

---------

Co-authored-by: Eike Ahmels <eike.ahmels@tu-dortmund.de>
davidbitton pushed a commit to davidbitton/betaflight that referenced this pull request Feb 5, 2024
* fix led strip for h7

* account for cloud build custom defines

* remove unused code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

None yet

8 participants