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

Add target SKYSTARSF722MINIHD #8794

Merged

Conversation

shellixyz
Copy link
Collaborator

No description provided.

@shellixyz shellixyz added this to the 6.0 milestone Feb 12, 2023
#define TARGET_BOARD_IDENTIFIER "SS7D"

#ifdef SKYSTARSF7MINIHD
#define TARGET_BOARD_IDENTIFIER "SSF7MHD"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't TARGET_BOARD_IDENTIFIER supposed to be 4 characters only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it is a problem. Some target board identifiers are longer. I think the longest one is the one for the iFlight Blitz F7 Pro which is 12 chars long

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we can use SS7D for one and SS7H for the other, to avoid issues with the TARGET_BOARD_IDENTIFIER.

Looking over the fence, on the BF side, it is just an identifier for the mcu used (makes sense for unified targets).

We can probably drop the uniquines requirement in INAV and change it to either be a 4 byte manufacturer id, or platform type like bf.

The TARGET_BOARD_IDENTIFIER is used in msp messages (limited to 4 bytes) and in crsf telemetry's device info. The crsf device info is just a string and could use USBD_PRODUCT_STRING instead.

@stronnag
Copy link
Collaborator

Noting that MSP_BOARD_INFO only sends four bytes, so if a consumer were to depend on unique IDs then longer IDs not unique in the first four characters may cause ambiguity.

@mmosca
Copy link
Collaborator

mmosca commented Apr 11, 2023

It looks like this wouldn't be the first target to exceed the recommend size for TARGET_BOARD_IDENTIFIER, so it should at least not be a blocker for merging.

Since this missed the 6.0 train, should we target it to 6.1?

Fyi: these all have board identifiers that are too long: TMOTORF7V2, IFLIGHT_BLITZ_F7_PRO, RUSH_BLADE_F7, KROOZX, AXISFLYINGF7PRO, YUPIF4, IFLIGHT_JBF7PRO, TMOTORF7V2, IFLIGHT_BLITZ_F722, TMOTORF7.

@mmosca mmosca modified the milestones: 6.0, 6.1 Apr 11, 2023
@mmosca
Copy link
Collaborator

mmosca commented Apr 12, 2023

@shellixyz can you target release_6.1.0? There will probably be a 6.1.0 release candidate soon.

@shellixyz shellixyz changed the base branch from master to release_6.1.0 April 12, 2023 21:30
@shellixyz shellixyz force-pushed the target/SKYSTARSF722MINIHD branch 2 times, most recently from fc1ca01 to 5ca30e9 Compare April 12, 2023 21:32
@shellixyz
Copy link
Collaborator Author

Done. I also changed the TARGET_BOARD_IDENTIFIER to be only 4 chars

@shellixyz shellixyz merged commit 74cbcb4 into iNavFlight:release_6.1.0 Apr 13, 2023
@MrD-RC
Copy link
Collaborator

MrD-RC commented Apr 14, 2023

Aren’t you supposed to be having a break 🤣

sensei-hacker pushed a commit to sensei-hacker/inav_unofficial_targets that referenced this pull request Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants