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 code base to ES6 #34

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

Update code base to ES6 #34

wants to merge 2 commits into from

Conversation

Jozo132
Copy link
Contributor

@Jozo132 Jozo132 commented Jan 30, 2020

**Includes ADS indexOffset fix caused 'read' 'write' and 'notify' to return values of 0 or false
Added '// @ts-check' for improved type checking
Replaced all 'var' types to 'const' and 'let' respectively
Replaced all 'function' types to 'const' arrow functions
Replaced all function inheritences of 'this' to pass as first parameter instead
Reduced code size by about 350 lines and structural imrpovements

Code examples must be updated becuase 'this' has to be replaced with your 'client' object
Example:

let client = ads.connect(amsConfig, () => {
    console.log(`Trying to read symbol:`, sym)

    //* // Read symbol
    client.read(sym, (err, handle) => {
        if (err) console.log(err)
        console.log('Response:', handle.value)
        client.end()
    })//*/

    /* // Add symbol to notify
    client.notify(sym)
    client.on('notification', handle => {
        console.log('Response:', handle.value);
    });//*/

})
client.on('error', e => {
    console.log(e)
})

**Includes ADS indexOffset fix caused 'read' 'write' and 'notify' to return values of 0 or false
Replaced all 'var' types to 'const' and 'let' respectively
Replaced all 'function' types to 'const' arrow functions
Replaced all function inheritences of 'this' to pass as first parameter instead
Reduced code size by about 350 lines and structural imrpovements

Code examples must be updated becuase 'this' has to be replaced with your 'client' object
Example:
```js
let client = ads.connect(amsConfig, () => {
    console.log(`Trying to read symbol:`, sym)

    //* // Read symbol
    client.read(sym, (err, handle) => {
        if (err) console.log(err)
        console.log('Response:', handle.value)
        client.end()
    })//*/

    /* // Add symbol to notify
    client.notify(sym)
    client.on('notification', handle => {
        console.log('Response:', handle.value);
    });//*/

})
client.on('error', e => {
    console.log(e)
})
```
@roccomuso
Copy link
Owner

Rebase needed.
Also, We might want to support old node versions.

@Jozo132
Copy link
Contributor Author

Jozo132 commented Feb 4, 2020

Rebase needed.
Also, We might want to support old node versions.

First time I'm doing rebase .. I think I done it right. // Inexperienced GIT user here
I agree with the support for old node versions, so do as you see fit.
I'll be working on my fork and will do some more testing when I have spare time.
What I needed was ADS support for latest node versions on Windows systems, so that's how it got me here.

@roccomuso
Copy link
Owner

cool keep us updated

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