ifrunistuttgart / node-mavlink

GNU Lesser General Public License v3.0
25 stars 13 forks source link

Import error in generated message-registry.ts #16

Open FullPint opened 4 years ago

FullPint commented 4 years ago

The following were generated using common.xml: enums/ messages/ message-registry.ts tsconfig.json

Inside of message-registry.ts:

import {MAVLinkMessage} from 'node-mavlink';

TypesScript gives the following error: Cannot find module 'node-mavlink'.ts(2307)

When changing the line to import @ifrunistuttgart/node-mavlink instead of node-mavlink:

import {MAVLinkMessage} from '@ifrunistuttgart/node-mavlink';
...
/*
   All lines following the export now throw a complaint
*/
export const messageRegistry: Array<[number, new (system_id: number, component_id: number) => MAVLinkMessage]> = [

The type error where {MAVLink type} is any type coming from common.xml:

Type 'typeof {MAVLink Type}' is not assignable to type 'new (system_id: number, component_id: number) => MAVLinkMessage'.
  Type 'Heartbeat' is missing the following properties from type 'MAVLinkMessage': _system_id, _component_id, _payload_length, _extension_length, and 2 more.ts(2322)
FullPint commented 4 years ago

For more information:

This came with the mavlink installation from their guide: The pymavlink version = '2.4.8'

And I was able to fix it by replacing all statements:

import {MAVLinkMessage} from 'node-mavlink';

with:

import {MAVLinkMessage} from @ifrunistuttgart/node-mavlink

I'm not sure if this is the wrong place to add the issue as it may belong with MAVLink/pymavlink

rummanwaqar commented 4 years ago

I ended up adding a line to my mavlink generation script to find and replace from 'node-mavlink'; to from '@ifrunistuttgart/node-mavlink';

FullPint commented 4 years ago

@rummanwaqar I ended up doing the same thing along with a few other quality of life things.

I also submitted an issue with that repository as well.

rummanwaqar commented 4 years ago

@FullPint Yes, I saw your issue on pymavlink repository. Thanks for making that. I might submit a PR for it.

What other QoL changes did you end up making? Just curious :)

FullPint commented 4 years ago

@rummanwaqar mostly hoisting types/enums up to the top level for importing.

There are also some issues with this package and the way it handles strings. It assumes any char type has a length of one.

rummanwaqar commented 4 years ago

That isn't great. You are right. Just ran a test with this message:

<message id="523" name="DEBUG_MSG">
      <description>Debug Message</description>
      <field type="uint32_t" name="timestamp">ms since startup</field>
      <field type="uint8_t" name="severity">Severity of message</field>
      <field type="char[249]" name="message">message payload</field>
    </message>

Then in TS

const mavlinkMessage = new DebugMsg(0, 255);
mavlinkMessage.message = "hey";

I run the message through a pack and parse cycle and the returned message has only the first character. Any fixes?

FullPint commented 4 years ago

@rummanwaqar Internally we forked it due to the lack of response here and with the other repository you saw I had submitted the other issue.

Off the top of my head there are some switch statements that exist with both packer and parser. I wrote a patch to properly encode/decode. Where the packer looks at the length of the string and encodes based on that, and the parser reads the length of the message and decodes based on that.

rummanwaqar commented 4 years ago

Thanks a lot @FullPint. Will probably just fork it.

FullPint commented 4 years ago

@rummanwaqar No problem. Feel free to shoot me a message if you run into any trouble.

Eventually we plan on writing our own, but with the current state of our project its a little bigger than we have time to take on.

rummanwaqar commented 4 years ago

@FullPint sounds good. Maybe we can collaborate

jquesada2016 commented 3 years ago

I found a work around that does not involve renaming anything. Just run this command to add node-mavlink as an alias.

npm i node-mavlink@npm:@ifrunistuttgart/node-mavlink

Was stuck on this for a bit, but that solved it for me.