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

AP_MSP: fix for multiple backends initialization #15757

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

yaapu
Copy link
Contributor

@yaapu yaapu commented Nov 9, 2020

A fix for multiple MSP backends initialization.

related issue #15756

@@ -102,7 +102,7 @@ void AP_MSP::init()
for (uint8_t protocol_instance=0; protocol_instance<MSP_MAX_INSTANCES-_msp_status.backend_count; protocol_instance++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

insn't the issue just :
for (uint8_t protocol_instance=_msp_status.backend_count; protocol_instance<MSP_MAX_INSTANCES; protocol_instance++)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mhmm I don't think so, shouldn't we check each serial protocol for all instances starting at 0 up to the MAX allowed?

@@ -86,7 +86,7 @@ void AP_MSP::init()
for (uint8_t protocol_instance=0; protocol_instance<MSP_MAX_INSTANCES; protocol_instance++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if we don't use the protocol_instance variable, maybe using

Suggested change
for (uint8_t protocol_instance=0; protocol_instance<MSP_MAX_INSTANCES; protocol_instance++) {
while (_msp_status.backend_count < MSP_MAX_INSTANCES) {

would be better ?

Copy link
Contributor Author

@yaapu yaapu Nov 10, 2020

Choose a reason for hiding this comment

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

@khancyr I refactored and removed backend_instance, now should be ok. It loops thru all instances per each serial protocol starting from instance 0 up to MAX instances accounting for already found backends. But is suboptimal 'cause it still looks for instance 1 even if instance 0 was not found, maybe the extra variable was needed after all

@khancyr khancyr added the Library label Nov 9, 2020
@yaapu yaapu force-pushed the pr/msp_fix_multiple_backends branch from 46f1b4f to 7190790 Compare November 10, 2020 06:58
@yaapu yaapu added the DevCallEU label Dec 8, 2020
@tridge tridge merged commit 42b85c8 into ArduPilot:master Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants