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

Update required to get Parrot Mambo Bebop working #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jwvictor
Copy link

This field was undefined for whatever reason, so handled for that case. It still finds that the other ID starts with Mambo_ so not a huge deal, but caused it to crash with my device (Parrot Mambo Bebop Minidrone). Thought you might like the update.

This field was undefined for whatever reason, so handled for that case. It still finds that the other ID starts with Mambo_ so not a huge deal, but caused it to crash with my device (Parrot Mambo Bebop Minidrone).
@fetherston
Copy link
Owner

@jwvictor thanks for the update! However, please setup eslint in your text editor and fix the changes to match the repositories lint rules (Note the failing build step in github).

@fetherston
Copy link
Owner

The command "npm test" exited with 0.
1.55s$ ./node_modules/.bin/eslint-changes
/Users/travis/build/fetherston/npm-parrot-minidrone/lib/MiniDroneBtAdapter.js
  407:9   error  Expected space(s) after "if"                              keyword-spacing
  407:22  error  Missing space before opening brace                        space-before-blocks
  408:17  error  Expected indentation of 12 space characters but found 16  indent

@jwvictor
Copy link
Author

@fetherston Thank you so much for having a look, would love to commit to your project even in some small form -- I have had so much fun with it already, I certainly owe you :) I don't write much JS so I'll grab a copy of eslint and resubmit the PR as soon as I can. Thanks again!

@jwvictor
Copy link
Author

@fetherston put in the changes the linter was asking for, CI should pass but it's taking a while :) lemme know if there's any trouble!

@@ -403,7 +403,10 @@ class MiniDroneBtAdapter extends EventEmitter {
const manufacturer = peripheral.advertisement.manufacturerData;
const matchesFilter = localName === this.options.droneFilter;

const localNameMatch = matchesFilter || DRONE_PREFIXES.some((prefix) => localName.indexOf(prefix) >= 0);
Copy link
Owner

@fetherston fetherston Jul 18, 2017

Choose a reason for hiding this comment

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

I don't have a Mambo Bebop, but I think there might be a more elegant way to fix this. What is the values of the peripheral.advertisement object at this line? Does peripheral.advertisement.localName start with Mambo_? Or something else?

If it's something else you can fix this by just adding it to DRONE_PREFIXES at the top of this class

Copy link
Author

Choose a reason for hiding this comment

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

localName was undefined at this point, but by defaulting localNameMatch to false it still caught it (presumably via the manufacturer match). I can print out the exact object when I get home if that'd help.

Copy link
Author

Choose a reason for hiding this comment

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

@fetherston It turns out what's happening is there are other (non-matching) devices in my house that have localName undefined. This causes an NPE on the .indexOf call. When the proper peripheral is actually found, localName is indeed defined and matches Mambo_. So, this is a general problem for any situation in which one tries to connect to the drone in the presence of other devices not broadcasting a localName. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants