pymavlink icon indicating copy to clipboard operation
pymavlink copied to clipboard

Published 20 hours ago • verified publisher icon ArduPilot

Fix Typescript generation to work with node-mavlink

Open JoshuaHintze opened this issue 2 years ago • 10 comments

While recently testing the typescript generation with the latest node-mavlink we found a number of issues. For example

  • multi-line comments in the XML show up only as single line
  • incorrect exports/imports
  • other misc differences

This is now working with the current head of node-mavlink for us.

NOTE

Added in a helper utility method for generating a list of all messages for use with node-mavlink custom message support. This new exported function is generated in message-registry.ts

Also adjusted based on comments.

JoshuaHintze avatar Feb 15 '23 00:02 JoshuaHintze

Just want to express my support for this contribution, as the current typescript generated library does not even work anymore. Me and my team would love to have this PR merged!

BearToCode avatar Mar 23 '23 19:03 BearToCode

Hello, is there any repo maintainer willing to take the time to look at this PR? It's kind of a big deal for me to have it merged in the repo... Maybe @tridge ? (sorry for the mention but you are the only active repo admin I was able to find, as there's no list of collaborators)

BearToCode avatar Mar 29 '23 09:03 BearToCode

@davidbuzz I think you are JS/TS guy, any opinion on this ?

khancyr avatar Mar 29 '23 12:03 khancyr

Based upon feedback from this node-mavlink issue disscussion (https://github.com/ArduPilot/node-mavlink/issues/21) we have this working correctly with custom messages now. I've updated the PR description.

JoshuaHintze avatar Mar 29 '23 21:03 JoshuaHintze

Hello, any news?

BearToCode avatar Mar 31 '23 15:03 BearToCode

@davidbuzz Are you able to approve?

JoshuaHintze avatar Apr 03 '23 21:04 JoshuaHintze

The changes this pr makes in the generator/javascript/ folder should NOT be merged... thats a different generator['javascript' vs 'typescript'], please remove them from this pr. As for the typescript changes, i cant comment, as i maintain the other one. ;-)

davidbuzz avatar Apr 13 '23 03:04 davidbuzz

Thanks @davidbuzz - I'm not sure how those got in there, but I will revert those two file changes. Do you know who can review the typescript changes?

JoshuaHintze avatar Apr 13 '23 03:04 JoshuaHintze

I dont know if we have a maintainer for 'typescript' bindings, but 'git blame' them to see whos made changes b4, or who created them in the first place... or failing that, convince the rest of us with testing evidence that its better.. ;-)

davidbuzz avatar Apr 13 '23 03:04 davidbuzz

Thanks @davidbuzz - I'm not sure how those got in there, but I will revert those two file changes. Do you know who can review the typescript changes?

Ping @pascalgross - from 2019... if we don't get a response we'll probably just welcome you as the new typescript generator maintainer :-)

peterbarker avatar Apr 13 '23 03:04 peterbarker