-
Notifications
You must be signed in to change notification settings - Fork 11
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
Vbet device not supported #111
Comments
hi, this productID is indeed not included in the current list. We recently redo some of the produtID and the supported device list should be updated accordingly, is it ok if we extract the code into a package and reference it from this project @maxwellmooney13 ? |
Hello @JunzheFan. Thanks for your feedback. |
@Draginfable hi, I opened a PR for it, can you test if the device you have can function |
Hello @JunzheFan, thanks a lot for your work. However, I see that if I try to build the module, it fails.
This is the error:
On latest develop the build works. Could you please take a look? Note 1: Build only works with node 16. It fails with node 18
|
hi @Draginfable thanks for the testing and feedback! The error was due to missing TS definition for the w3c-WebHID APIs, so I changed the package API a bit, now when I run |
Thanks @JunzheFan!
|
Hi all, I apologize for not being responsive on this. We've been pulled into a number of other things that took our attention. @Draginfable thank you for raising the issue and bringing this to our attention and @JunzheFan thank you for raising a PR to hopefully handle this sorta stuff a lot better. So the original issue is handled with the PR branch but the issue now is related to builds? I'll take a look today and see if I can make sense of it. |
Correct, on master I can build successfully using node 16 (not with 18), but if you checkout this branch the module build fails with both. It could be the same root cause. Please let me know if I can help. |
@Draginfable @maxwellmooney13 I got different error on Terser plugin when building, but from the error shown above, it seemed the error was due to webpack is not configured to process mjs extension file, so I updated the package to output only js extension file for ES6, can you guys install again to test if error go away ? |
Unfortunately, I still get this error. It could be something on my environment though. |
@Draginfable can you confirm the error is still the unparsed extension |
|
softphone-vendor-headsets-develop npm run build:module
build mode: development ERROR in softhphone-vendor-headsets.js from Terser ERROR in 1.softhphone-vendor-headsets.js from Terser ERROR in 2.softhphone-vendor-headsets.js from Terser ERROR in 3.softhphone-vendor-headsets.js from Terser |
On develop I get an error with node 18. But it works with node 16. |
@Draginfable @maxwellmooney13 hello, I found out the issue was because the vbet package was not transpile to es5 because the plugin it used does not support producing es5 egoist/rollup-plugin-esbuild#364, so I use the webpack babel loader in your project to transpile instead by changing the webpack config this line to |
Hi all! Once again, I'm sorry for a lack of response here... I've had a lot of things come up that required attention. Great job finding a solution for the build issue! I'll look at the changes and double check to make sure it's okay. |
Pulling down the branch now to check |
Hey all, you guys probably already figured this out yourselves but with the branch locally, just making the change @JunzheFan suggested in the webpack config was not enough (however that change does fix the error mentioned regarding an appropriate loader being needed). I still had a bunch of Tenser related errors which were fixed by running |
@JunzheFan is the |
@maxwellmooney13 hello, yes indeed the package itself is not transpiled(i.e. no babel tools is configured) the output bundled code stay at modern es2015/es6 format which is not applicable to very old browswer, thus I need to resort to the webpack babel loader in this project to do the transpileing work, or I have to find another way to pre-transpiled the code to fit within the softphone-vendor-headsets project so that in this case the |
Hi @JunzheFan I made a change that seems to work locally. In the
but adding in
I'm not sure if there is a better way to make this cleaner but this seems to work, at the very least it builds properly for me but I'll need to try with the demo app and such to see that we are in fact all good. |
@maxwellmooney13 hi, at the end of the day it comes to transpile the code down via babel, I have made a new version where babel is configured within the package itself so that webpack.config.js modification is not needed, can you pull down new package version and give it a try |
@JunzheFan I still had to run @Draginfable when you get a chance, I would like your opinion on the latest change too (more from a functionality standpoint) |
@maxwellmooney13 yes recent changes is all within the package, regarding the testing, I noticed something strange in the demo app that when you init a Incomming call, the |
Great job guys! Thanks a lot! EDIT: |
hi guys I have updated package-lock.json to the newest package version |
@maxwellmooney13 hi, the management is asking the estimated time the changes can get released to production ,do you have any idea, thanks |
@JunzheFan just merged your changes in. I'll need to make some time to go through the proper steps to get this into production but at this moment, your part in the change is done! I'll work on getting it to prod this week. Is this issue good to close now? |
Great news. Looking forward to the release! |
Hi all, just as a heads up, I'm trying to get the changes in for a release but we are having some branching issues with the repo. We'll be able to get the changes out as soon as we fix these. |
@maxwellmooney13 hey max, is this released or waiting for the pending PR you made ? |
Hi @JunzheFan I was bouncing between a few different things and got sidetracked from the PR. I'm going to merge my unit test PR to (hopefully) ensure proper builds of the release branch. I will do that tomorrow (tomorrow for me, later today for you!) and quickly get the release up and running. |
@maxwellmooney13 morning,can you inform us when the release is done ,we wanted to do a few testing today found that the changes might not have been applied ,also do you know what's required for the devices to function on the desktop version app |
hi @JunzheFan ! at the earliest, i can get it out this weekend but i will keep you up to date. i will work on this today. as for the desktop app, we are currnetly unable to use WebHID functionality on the desktop app. we are actively looking for a proper solution though this may take a lot more effort. |
@JunzheFan Actively working on getting this out. Facing some blockers on our end that are making our process a bit more difficult but wanted you to know I am still working on this. |
Hi all, I think we finally got things sorted out and good! The way our release schedule works, I think we should get this out this weekend I believe. I will keep everyone updated if something changes. As always, I apologize for this whole process taking way longer than it should have but we are in sight of the finish line now! Thank you for the work on this and the testing provided for the change. |
Just out of curiosity, I tested with v2.5.4 and the vbet headsets seem to work fine. Is something important missing from this release, or is it ok if we use this one until the final one is released? |
Device: VT8200 UNC DUO USB P/N: 8257-91-00UK
Product Id: 0x0028
This product id is not included in the masks / deviceCmds and none of these four masks seem to work.
cc @maxwellmooney13 @jensengar
Please let me know if I need to provide more information.
The text was updated successfully, but these errors were encountered: