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

add TypeScript definitions #176

Merged
merged 2 commits into from
Jul 20, 2018
Merged

Conversation

vaishnavachath
Copy link
Contributor

originally written by @troywweber7
also submitted to @types : DefinitelyTyped/DefinitelyTyped#26814

Copy link
Owner

@jadonk jadonk left a comment

Choose a reason for hiding this comment

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

It seems like each function returns an 'any' type. When called locally (non-RPC), each function has a possible return value. For RPC-called functions, I think it should always be 'void' or whatever the Typescript equivalent is.

src/index.d.ts Outdated

export function getPlatform(callback?: (error: ErrorType, platform: any) => void): any;

export function getEeproms(callback?: (error: ErrorType, eeproms: any) => void): any;
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be deprecated from the public interface.

src/index.d.ts Outdated
// FUNCTIONS - TODO obliterate all "any" types where possible! --- TWW
// https://github.com/jadonk/bonescript#system

export function getPlatform(callback?: (error: ErrorType, platform: any) => void): any;
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason that 'platform' is of type 'any' and not a particular structure?

src/index.d.ts Outdated

export function echo(data: any, callback?: (error: ErrorType, data: any) => void): any;

export function readTextFile(filename: string, callback?: (error: ErrorType, data: any) => void): any;
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't 'data' be of a string type?

src/index.d.ts Outdated

export function getEeproms(callback?: (error: ErrorType, eeproms: any) => void): any;

export function echo(data: any, callback?: (error: ErrorType, data: any) => void): any;
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't 'data' on both the call and callback be of a string type?

src/index.d.ts Outdated

export function readTextFile(filename: string, callback?: (error: ErrorType, data: any) => void): any;

export function writeTextFile(filename: string, data: any, callback?: (error: ErrorType, data: any) => void): void;
Copy link
Owner

Choose a reason for hiding this comment

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

Why does the callback have a 'data' value? I think this should only be returning a status of error or not error.

src/index.d.ts Outdated

export function writeCModule(filename: string, data: any, callback?: (error: ErrorType, data: any) => void): void;

export function setDate(date: any, callback?: (error: ErrorType, data: any) => void): void;
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this should be returning any 'data', just an error status. The 'date' should be a string type with specific formatting.

src/index.d.ts Outdated

export function pinMode(
pin: string,
direction: any,
Copy link
Owner

Choose a reason for hiding this comment

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

string.

src/index.d.ts Outdated
pin: string,
direction: any,
mux?: any,
pullup?: any,
Copy link
Owner

Choose a reason for hiding this comment

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

string

src/index.d.ts Outdated
direction: any,
mux?: any,
pullup?: any,
slew?: any,
Copy link
Owner

Choose a reason for hiding this comment

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

string. Several of these should probably should be a string enums if typescript supports that.

src/index.d.ts Outdated

export function getPinMode(pin: string, callback?: (err: ErrorType, mode: PinModeObj) => void): any;

export function shiftOut(dataPin: any, clockPin: any, bitOrder: any, val: any, callback?: ErrorCb): void;
Copy link
Owner

Choose a reason for hiding this comment

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

dataPin and clockPin should be strings.
val should be a number.
I forget what bitOrder should be.

src/index.d.ts Outdated
[i: string]: i2cInfo;
}

export interface PinInfo {
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure if we want to include full details here. They are handy, but I'm not 100% committed to keeping them all here. What ones do you think people actually use?

src/index.d.ts Outdated
universalName?: string;
}

export interface PwmInfo {
Copy link
Owner

Choose a reason for hiding this comment

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

Really don't think this should be a public interface. I should probably hide it.

src/index.d.ts Outdated
sysfs: number;
}

export interface UartInfo {
Copy link
Owner

Choose a reason for hiding this comment

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

Not really a public interface. Referenced only by string.

src/index.d.ts Outdated
tx?: string;
}

export interface i2cInfo {
Copy link
Owner

Choose a reason for hiding this comment

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

Not really a public interface. Referenced only by string.

sda?: string;
}

// TYPES - adjusted based on observation / testing
Copy link
Owner

Choose a reason for hiding this comment

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

Did these get used up above?

Copy link
Owner

Choose a reason for hiding this comment

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

Looks like this is a reasonable way to do string enums!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, most of them were used above, i have removed some of the exports which were not necessary, had to export them previously due to this rule: https://github.com/Microsoft/dtslint/blob/master/docs/strict-export-declare-modifiers.md

change function return value type to any
removed getEeproms definition
defined platform object structure
removed unnecesary exports
@vaishnavachath
Copy link
Contributor Author

Hi, i have tried to make the suggested changes, can you have a look at it

@jadonk jadonk merged commit 0c7c11f into jadonk:master Jul 20, 2018
@vaishnavachath
Copy link
Contributor Author

beagleboard#46

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