search

ShikOfTheRa / scarab-osd

MWOSD - UAV HUD
http://www.mwosd.com
423 stars 204 forks source link

KISS 1.3RC39b : size of KISSserialBuffer too small #573

Closed xaviergriffon closed 4 years ago

xaviergriffon commented 5 years ago

The size of the "KISSserialBuffer" array is too small with KISS 1.3RC39b. In my tests, the frame length was 170. I would advise using a size of 256 (KISSFRAMELENGTH) as KISS OSD does.

I will test flights tomorrow to validate that this is the only issue ;)

xaviergriffon commented 5 years ago

I tried other solution with a simple "if" when we stack buffer.

  else if (c_state == KISS_HEADER_SIZE) {
    if (Kvar.index < KISSFRAMELENGTH) {
      KISSserialBuffer[Kvar.index] = c;
    }

Currently, the last index (for Mah) is 148. This solution allows to use a smaller buffer (149).

ShikOfTheRa commented 5 years ago

It has grown over time :) Thanks for feedback. Not sure if many use MWOSD for KISS. I get that it has its own but would like to see it more widely utilised. KISS is awesome and rated #1 by the MWOSD GUI dev :)

Memory is in very short supply without some time consuming optimisation, but if any additional functionality needed to match what KISSOSD offers happy to look at it. I think dev on that has stopped?

So suggest:

in globalvariable.h change:

define KISSFRAMELENGTH 200 // max frame size

in serial.ino add:

elif defined KISS

define SERIALBUFFERSIZE 65

else

increase KISS data buffer significantly. Reduces MSP serial buffer (only used for GUI) by same amount

Net change = zero so should be no impact on functionality.

fedorcomander commented 5 years ago

Present for you...

// link stats, crossfire only for now

        kissProtocolWriteByte(RXLinkStats.upRSSI1, &dst);
        kissProtocolWriteByte(RXLinkStats.upRSSI2, &dst);
        kissProtocolWriteByte(RXLinkStats.upLQ, &dst);
        kissProtocolWriteByte(RXLinkStats.upSNR, &dst);
        kissProtocolWriteByte(RXLinkStats.upAntenna, &dst);
        kissProtocolWriteByte(RXLinkStats.rfMode, &dst);
        kissProtocolWriteByte(RXLinkStats.upTXPower, &dst);
        kissProtocolWriteByte(RXLinkStats.downRSSI, &dst);
        kissProtocolWriteByte(RXLinkStats.downLQ, &dst);
        kissProtocolWriteByte(RXLinkStats.downSNR, &dst);

You also can get gps data (via differenc command), lmk if interested.

Alex

xaviergriffon commented 5 years ago

I created a PR (#574 ) in which I left comments and definitions that I used during the analysis. No problem to delete them if you want or to make the simplest with the buffer at 200 and without if.

ShikOfTheRa commented 5 years ago

@ Alex

You also can get gps data (via difference command), lmk if interested. Alex

Definitely interested in the GPS data!

I don't use crossfire so that will be a challenge for me..

ShikOfTheRa commented 5 years ago

@Zavier - its added. Many thanks for that

fedorcomander commented 5 years ago

KISS_PROTOCOL_GET_GPS = 0x54,

case KISS_PROTOCOL_GET_GPS: kissProtocolWriteInt32((int32_t) (gps.latitude 10000000.0f), &dst); kissProtocolWriteInt32((int32_t) (gps.longitude 10000000.0f), &dst); kissProtocolWriteWord((uint16_t) (gps.speed 100.0f), &dst); kissProtocolWriteWord((uint16_t) (gps.course 100.0f), &dst); kissProtocolWriteWord((int16_t) gps.altitude, &dst); kissProtocolWriteByte((uint8_t) ((gps.fix << 7) | (gps.satellites & 0x7f)), &dst); resp_len = (uint16_t)(dst - &response[2]); break;

ShikOfTheRa commented 5 years ago

Brilliant - I will look to implement next week.

fedorcomander commented 5 years ago

Lmk if u need any help. buzz me for gps capable firmware if u need.

ShikOfTheRa commented 5 years ago

Is firmware available for V1 board? Guess not enough resources?

fedorcomander commented 5 years ago

sure it is... plenty of resources yet on v1...

fedorcomander commented 5 years ago

its not taking that much cpu, only adaptive filter is heavy, but hey, you do need to crunch that numbers ;)

ShikOfTheRa commented 5 years ago

Fantastic - can you drop a mail to xxxx and we can take this offline. It is cool development for KISS. seeing a lot more guys doing longer range and want GPS

error414 commented 5 years ago

It is great news, may I ask you for FW for KISS V1, does it need new configurator too? My email xxxx . I would like to test it, if it is possible.

fedorcomander commented 5 years ago

Yeah, i am hooked on LR now, so need to have GPS kiss goodies for myself ;)

ShikOfTheRa commented 5 years ago

Alex - error414 is the GUI dev..

fedorcomander commented 5 years ago

mailed you, you can distribute along your devs. please dont share, its my branch, not yet official.

xaviergriffon commented 5 years ago

@ShikOfTheRa: Thank you for integrating the correction. If you need any help for tests or implementation, do not hesitate to contact me by mail xxxx

For me, this issue can be closed.

ShikOfTheRa commented 5 years ago

Thanks both

Xavier - I'm going to keep open a little longer as a reminder for me to make sure I add in the GPS support.

Alex - yes agree no sharing outside the 2 core devs

fedorcomander commented 5 years ago

okidoki

Cheers, Alex

On 27 Aug 2019, at 11:44, ShikOfTheRa notifications@github.com wrote:

Thanks both

Xavier - I'm going to keep open a little longer as a reminder for me to make sure I add in the GPS support.

Alex - yes agree no sharing outside the 2 core devs

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

xaviergriffon commented 5 years ago

KISS_PROTOCOL_GET_GPS = 0x54,

case KISS_PROTOCOL_GET_GPS: kissProtocolWriteInt32((int32_t) (gps.latitude 10000000.0f), &dst); kissProtocolWriteInt32((int32_t) (gps.longitude 10000000.0f), &dst); kissProtocolWriteWord((uint16_t) (gps.speed 100.0f), &dst); kissProtocolWriteWord((uint16_t) (gps.course 100.0f), &dst); kissProtocolWriteWord((int16_t) gps.altitude, &dst); kissProtocolWriteByte((uint8_t) ((gps.fix << 7) | (gps.satellites & 0x7f)), &dst); resp_len = (uint16_t)(dst - &response[2]); break;

@fedorcomander, does KISS_PROTOCOL_GET_GPS instruction work in rc40i ? I tried (your sharing on RCGroups :)), but I have no answer, no more instructions work after for a very long time. I tried via the serial port and directly in USB but it's the same.

fedorcomander commented 5 years ago

yeah, i had to add this command to “backward compatibility” commands - single byters…. ;) fixed in 40j

On 6 Sep 2019, at 21:00, xaviergriffon notifications@github.com wrote:

KISS_PROTOCOL_GET_GPS = 0x54,

case KISS_PROTOCOL_GET_GPS: kissProtocolWriteInt32((int32_t) (gps.latitude 10000000.0f), &dst); kissProtocolWriteInt32((int32_t) (gps.longitude 10000000.0f), &dst); kissProtocolWriteWord((uint16_t) (gps.speed 100.0f), &dst); kissProtocolWriteWord((uint16_t) (gps.course 100.0f), &dst); kissProtocolWriteWord((int16_t) gps.altitude, &dst); kissProtocolWriteByte((uint8_t) ((gps.fix << 7) | (gps.satellites & 0x7f)), &dst); resp_len = (uint16_t)(dst - &response[2]); break;

@fedorcomander https://github.com/fedorcomander, does KISS_PROTOCOL_GET_GPS instruction work in rc40i ? I tried (your sharing on RCGroups :)), but I have no answer, no more instructions work after for a very long time. I tried via the serial port and directly in USB but it's the same.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ShikOfTheRa/scarab-osd/issues/573?email_source=notifications&email_token=AAQBCCXJ6EH32TUFANBMPNLQIKSFLA5CNFSM4IN5SH72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6DYLTQ#issuecomment-528975310, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQBCCQ7PEK7HAKQKCXX2HLQIKSFLANCNFSM4IN5SH7Q.

xaviergriffon commented 5 years ago

Thanks @fedorcomander and sorry to respond so late ... Is it important to switch all "get" requests to the format: "Command" "Size" "data" "CRC" ? I tried for the GPS and it works better :)

ShikOfTheRa commented 4 years ago

@ xaviergriffon - All the KISS items are released in 1.9.1.5

1.9.1.5 Summary of changes : OSD - Support for KISS GPS OSD - Support for KISS PID menu support OSD - Support for KISS Filter menu OSD - Support for KISS VTX menu OSD - Support for ARDUSUB OSD - Support for MAVLINK dual GPS / Baro altitude display OSD - Support for MAVLINK high resolution altitude OSD - Significantly improved Mavlink messages. Full length and human readable OSD - Different indicator icons for GPS, Baro and high resolution altitude values OSD - Bugfix for iNav airspeed display OSD - Bugfix for altitude display. GPS can be displayed independently of Baro altitude OSD - Developer debug memory display functioning for newer compiler versions

ShikOfTheRa commented 4 years ago

Closing. I think all KISS is working OK from feedback Also have support for Steele PDB.

Cool - many thanks Xavier

Re-open if need