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.

Sign up for GitHub

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

Jump to bottom

Platform target evaluation fix #12904

Merged
merged 1 commit into from Jun 21, 2023
Merged

Platform target evaluation fix #12904

merged 1 commit into from Jun 21, 2023

Conversation

ASDosjani
Copy link
Contributor

@ASDosjani ASDosjani commented Jun 20, 2023 •
edited

Thanks to @haslinghuis who noticed that UNIFIED_SERIAL_PORT_COUNT was evaluated only after it was used so it might result UNDEFINED. I removed it because it was only used in target.h
Also changed the old Softserial defines. See #11786

@github-actions
Copy link

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

  • Simply put #12904 (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!

@haslinghuis haslinghuis added this to the 4.5 milestone Jun 20, 2023
@blckmn
Copy link
Member

blckmn commented Jun 20, 2023 •
edited

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> PASS
  • 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

blckmn
blckmn reviewed Jun 20, 2023
View reviewed changes
src/main/target/STM32F405/target.h Outdated
#define USE_UART1
#define USE_UART2
#define USE_UART3
#define USE_UART4
#define USE_UART5
#define USE_UART6

#define SERIAL_PORT_COUNT (UNIFIED_SERIAL_PORT_COUNT + 6)
#define SERIAL_PORT_COUNT 9
Copy link
Member

Choose a reason for hiding this comment

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

Confirming this number is correct? Should it include VCP etc?

Copy link
Contributor Author

@ASDosjani ASDosjani Jun 20, 2023 •
edited

Choose a reason for hiding this comment

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

I talked about it with @haslinghuis and I fixed it now.

Edit: It can't be compiled without including those.

Copy link
Member

Choose a reason for hiding this comment

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

As UNIFIED_SERIAL_PORT_COUNT only was defined after being referenced VCP, SOFTSERIAL and LPUART were not included before. It seems to include LPUART.

@ASDosjani ASDosjani force-pushed the evaluate-fix branch 2 times, most recently from 3858c4b to e8f1e4a Compare June 20, 2023 20:11
@haslinghuis haslinghuis merged commit c243e83 into betaflight:master Jun 21, 2023
20 checks passed
@juanxli juanxli mentioned this pull request Jun 23, 2023
Closed
freasy pushed a commit to freasy/betaflight that referenced this pull request Jul 19, 2023
@ASDosjani @freasy
Platform target evaluation fix (betaflight#12904)
c178c9d
davidbitton pushed a commit to davidbitton/betaflight that referenced this pull request Feb 5, 2024
@ASDosjani @davidbitton
Platform target evaluation fix (betaflight#12904)
4ec8d3f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blckmn blckmn blckmn approved these changes

@haslinghuis haslinghuis haslinghuis approved these changes

@ctzsnooze ctzsnooze ctzsnooze approved these changes

Assignees

@ASDosjani ASDosjani

Labels
RN: BUGFIX
Projects
Finalizing Firmware 4.5 Release
Status: COMPLETED
Milestone
4.5
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ASDosjani @blckmn @haslinghuis @ctzsnooze