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

fix: add checks for peripheral.mtu #165

Merged
merged 1 commit into from
Mar 30, 2021
Merged

fix: add checks for peripheral.mtu #165

merged 1 commit into from
Mar 30, 2021

Conversation

mylylyl
Copy link

@mylylyl mylylyl commented Jan 24, 2021

checks if it's valid when setting it.
Should fix the TypeError of it

@@ -569,7 +569,7 @@ Noble.prototype.onHandleNotify = function (peripheralUuid, handle, data) {

Noble.prototype.onMtu = function (peripheralUuid, mtu) {
var peripheral = this._peripherals[peripheralUuid];
peripheral.mtu = mtu;
if (peripheral && peripheral.mtu && mtu) peripheral.mtu = mtu;
Copy link

Choose a reason for hiding this comment

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

isnt "if (peripheral)" enough ?

Copy link

Choose a reason for hiding this comment

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

or maybe : var peripheral = this._peripherals[peripheralUuid] || {} ?

Copy link
Author

Choose a reason for hiding this comment

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

or maybe : var peripheral = this._peripherals[peripheralUuid] || {} ?

if peripheral = {} then peripheral.mtu will get TypeError i guess? same for if (peripheral) when accessing .mtu

Copy link

Choose a reason for hiding this comment

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

if peripheral = {} then peripheral.mtu is undefined but you can do affectation:

``
node
Welcome to Node.js v14.15.0.
Type ".help" for more information.

var p = {};
undefined
p.foo = true;
true
console.log( p.as)
undefined

@rzr
Copy link

rzr commented Feb 14, 2021

please rebase

@rzr
Copy link

rzr commented Mar 12, 2021

can you please try to update your branch ?

checks if it's valid when setting it.
Should fix the TypeError of it
@mylylyl
Copy link
Author

mylylyl commented Mar 17, 2021

@rzr sry for the late reply just rebased it

Copy link

@rzr rzr left a comment

Choose a reason for hiding this comment

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

could be simplified

Copy link
Author

@mylylyl mylylyl left a comment

Choose a reason for hiding this comment

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

But this is to fix TypeError. Checked with typescript and yields error

@rzr rzr merged commit 0ab50b2 into abandonware:master Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants