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 support for APsystem micro-converters using ECU-R #176

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

rkokkelk
Copy link

Continued PR from #175 . Due to rebase on different branch new PR had to be created. Main difference is that this PR is based on the sdk-v3 branch instead of master.

With the discussion based on the separation of responsibility in the apsystems module. I'm not looking forward to duplicating basically the same code and having to maintain it in two separate locations. I understand your maintainability point of view, how about I give you direct access to the repo so that you can modify it as you see fit. So that the code is not duplicated but you can still modify for maintainability of this app.

@DiedB
Copy link
Owner

DiedB commented Nov 27, 2021

Are you sure this compiles? The TypeScript compiler seems to find quite a few issues (see 'files changed', TypeScript errors are embedded in there).

APsystems is not defined in Typescript and therefore does not contain a
type specification. In order for TS to compile properly `alltypes.d.ts`
is defined to define apsystems module.
@rkokkelk
Copy link
Author

There were indeed some issues which should now be resolved. Some issues mentioned by tsc which are shown in 'files changed' were due to typescript not detecting inheritance. I've fixed those by running:

npm i --save-dev @types/node

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