solved the logical error in grouping of escs into 4 #25156
Conversation
| const uint8_t num_idx = ESC_TELEM_MAX_ESCS/4; | ||
|
|
||
| // ensure we send out partially-full groups: | ||
| const uint8_t num_idx = (ESC_TELEM_MAX_ESCS % 4 == 0) ? ESC_TELEM_MAX_ESCS / 4 : (ESC_TELEM_MAX_ESCS / 4) + 1; |
There was a problem hiding this comment.
| const uint8_t num_idx = (ESC_TELEM_MAX_ESCS % 4 == 0) ? ESC_TELEM_MAX_ESCS / 4 : (ESC_TELEM_MAX_ESCS / 4) + 1; | |
| const uint8_t num_idx = (ESC_TELEM_MAX_ESCS + 3) / 4; |
There was a problem hiding this comment.
yes, Peters suggestion is correct
There was a problem hiding this comment.
Thanks for the review. Sure! I will rectify and update it.
|
Thanks for this. Can you also modify the commit description to start with "AP_ESC_Telem:"? |
|
@adityaomar3 are you still interested in pursuing this one? |
Sure I will amend it. |
Yes, I think small changes are needed, I will complete that shortly. |
adityaomar3
force-pushed
the
ap-esc
branch
2 times, most recently
from
e06399e
to
1fe056a
Compare
|
Hi! I was trying to squash the commits to 1 but many commits have been added since first commit so they get merge with the latest ones, can I get some help about this or squashing is not needed at all? I am still a novice. |
|
best bet is to make a new branch based on ardupilots master, then cherry pick the original commit and the update commit on to it. Then do a interactive rebase to squash the two commits into one. (see https://ardupilot.org/dev/docs/git-interactive-rebase.html). Then once your happy with that branch you can push it over this one to update the PR. If you get stuck we can always do it for you. |
|
Hi! Although it compiled in my pc earlier but failing some checks I was trying to understand the failing checks, but got a little understanding, can I get some help for better understanding and debugging. |
|
@adityaomar3 Sometimes the tests fail randomly, I have started them again, hopefully they will pass this time. |
ESC_Telemetry will not send MAVLink MSG_ESC_TELEMETRY_1_TO_4 if ESC_TELEM_MAX_ESCS (PWM/SERVO) count is <=3 as logic was to keep idx = ESC_TELEM_MAX_ESCS/4 It gives rise to 3 big issues that are :-
1.It will leave some values if ESC_TELEM_MAX_ESCS which are not completely divisible by 4 e.g. 1,2,3,5,6,7,9....
2. (next_idx + idx) % num_idx it will give error if escs are less than 4
3. for loop will never start if escs count is less than 4 i.e the message inside loop will be skipped even if escs are present. Also if escs are not in multiple of 4 then it will skip 1 iteration
Solution in commit:
This is rectified by logic to add 1 to num_idx if ESC_TELEM_MAX_ESCS is not multiple of 4.