-
Notifications
You must be signed in to change notification settings - Fork 14
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
Refactor function buttons #71
Conversation
Hi David. I would like to get these changes implemented. I haven't had time to look at all this. I want turnouts to work and to incorporate EX-RAIL buttons to handle routes. Can you help? |
@FrightRisk I'd be happy to help out. I can start by implementing the [EX-RAIL] (https://dcc-ex.com/automation/EX-RAIL-summary.html#DIAGNOSTICS-Control) commands into the command library. |
Ok, will do
…On Sun, Feb 27, 2022, 3:44 PM David Young ***@***.***> wrote:
@FrightRisk <https://github.com/FrightRisk> I'd be happy to help out. I
can start by implementing the [EX-RAIL] (
https://dcc-ex.com/automation/EX-RAIL-summary.html#DIAGNOSTICS-Control)
commands into the command library.
It would be great if you could share examples of what the return values
would look like. Especially for the Routes and Running tasks commands.
—
Reply to this email directly, view it on GitHub
<#71 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI36OWAWA6V6SSY5RSRJYWTU5KEK7ANCNFSM5KJG4VEA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@FrightRisk I've opened a PR to add the EX-Rail commands to the command library, if you wouldn't mind giving it a once over. I've also opened an issue to add the EX-Rail parsers, if you have any further details on what they look like. I've still to update this PR 🙃 |
Thank you! I will look
Fred
…On Sat, Mar 12, 2022, 12:58 PM David Young ***@***.***> wrote:
@FrightRisk <https://github.com/FrightRisk> I've opened a PR
<cloudthrottle/dcc-ex--commands#5> to add the
EX-Rail commands to the command library, if you wouldn't mind giving it a
once over.
I've also opened an issue
<cloudthrottle/dcc-ex--commands#6> to add the
EX-Rail parsers, if you have any further details on what they look like.
I've still to update this PR 🙃
—
Reply to this email directly, view it on GitHub
<#71 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI36OWCVQOMUQYTHGNWEIKLU7TLKPANCNFSM5KJG4VEA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@dcyoung-dev It is good that some real developer looking at this. Thank you for contributing. |
If you look at the conflicts, does it look like we can just get rid of lines 126 to 135 and keep all that is above it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me from a developer standpoint and simplify some of the function implementation. This will need to be tested before it is released directly in to the master branch. I can attempt a test sometime before Monday if that is ok. Other than that this is a good stepping stone before we move a JS framework like Vue or React or what have you. Thanks for the PR
Our two main challenges are that it is difficult on this project so far to find a person or person's who will commit to doing the work, and if we find someone who wants to do it using a tool they like, we have narrowed the pool of people who can pick up the project later. I hope we can reach critical mass with WebThrottle-EX in that we have enough people who are willing to contribute, or even one or two who stay with it for a few years and document it, we don't get caught unable to keep moving te product forward. |
In my tests, the buttons no longer work. I clicked on the pr and downloaded the zip from @dcyoung-dev 's repo. The buttons are not press-able and then don't execute functions. Everything else as far as coding looks good. Can we get that fixed? Next question is if we need to move to Vue.js |
Moves methods to a Javascript module
Rearranged toggling and command generation
Removes multiple `if` statements
Creates a `toggleButtonState` method
Move JS libraries
* Updated Command library * Updated branch to latest `master`
556ff19
to
296b9e7
Compare
Should all be fixed in 296b9e7
I've been playing around with a React implementation that got wildly complex. State management is the big issue as far as I can see. React or Angular would be up to the task of handling a lot of state changes, I'm not sure how well Vue holds up? |
I am open to any tool that does the job and can be documented so if someone leaves the project, others can follow |
Still doesn't work. I go to your repo, download the zip, unzip it to a folder and run index.
Basically, all that I can do is connect to the serial port, slide the power button (which does nothing), enter a loco ID, and press the button, and the throttle comes alive but no commands are sent when interacting with it. I also think that if you enter a debug command and press send, the command should not remain in the command window, what do you think? It would be kinda cool if it acted like a cmd box in windows where the command is "sent", but you can use the up and down arrows to scroll through the last several commands you entered. |
As far as switching VueJS is the easiest of all the frameworks to learn. React and Angular have a very steep learning curve. As far as state management that is what Vuex is for it's easy to modify. You can even get updates via subscriptions if you so choose. I use Vue for all my sites now. |
I'm not sure what's going on @FrightRisk as I do not see the same behaviour. The only thing that I have slightly different is that I've disabled the Service Worker as it was getting in the way while developing. As for the discussion of whether to Vue or not to Vue it would be best to keep it all in one place #68 |
@FrightRisk which browser are you using? |
Could I be doing something wrong on my end? I go here: https://github.com/dcyoung-dev/WebThrottle-EX/tree/refactor-function-buttons. Click on the "code" button to download the zip. I unzip it in my download folder. I double click on "index.html" and it loads in chrome. I see the index.html file has the lang=en, but maybe I still don't have the latest version? Previous versions of WebThrottle work fine. |
Chrome Version 101.0.4951.67 (Official Build) (64-bit). Windows 10. I see no errors. But I did determine the first issue of buttons not depressing is something with the metallic theme. When I switch to simple or dark, the buttons push. But They still don't show anything in the log when I press them. Also, I can't disconnect. Once connected, the button will not work anymore. |
How are you making that large log on the right? We need to fix the log window to size with the screen. 3 or 4 log lines just isn't enough. |
You should put your name in the credits and bump the version. That way I will know if I have the right code running. I also noticed is it installed as a progressive web app or when I look in the settings. There is no option to uninstall it, so I'm not sure if that has anything to do with the issues. But there are all sorts of issues. I was seeing throttle commands when I moved the slider, but nothing I do now shows them. I switched to simple theme and slider. I pressed stop and then tried to work the slider. It did not move, yet the numbers showing throttle were increasing moved with the mouse. I am having trouble duplicating that one now. |
@FrightRisk it sounds like there are a lot of issues to sort through. Have you tried downloading the
This is the browser's developer tool which you can get to by right-clicking anywhere in the UI and selecting Inside the developer tool, you can select "application" where you would be able to "Unregister" the Service Worker. Refreshing the page after registering should load the latest files. Again from the developer tool, you can view the Javascript files that have been loaded - even looking into the code inside them. Looking at the code you should see the changes that I have made, if not, you have stale code running, if you see the changes I'll dig further into the issue |
I will test this branch out when I get home tonight to see if I can replicate the issue that is experienced by Fred maybe I can shed some light and troubleshoot the issue. It's been a while since I updated the webthrottle anyways. Me and Mani did start a flutter based project for the webthrottle that was supposed to be fully cross platform we still have the code up if you guys wanna look at it as well. Flutter is a great framework as well and would allow us to use native app support on iOS and Android |
IssueI believe my issue is because I run from my folder and not from a web server. Testing 1.3 released versionWorks perfectly despite 2 errors showing up in the console log as shown below in Master that has the same 2 errors. Testing MasterMaster does not work either, though the buttons depress and I see output from interacting with the GUI before I try to connect to an Arduino. There are 2 errors in master before trying to connect:
and this:
But once I connect, nothing happens in our log. It seems as if there are no outgoing messages because the "writer" fails. If I inspect the console after trying to connect, I get a lot more errors. Testing Re-Factor function buttons versionI see this under the "issues" tab in the inspector. Though this also appears on all versions, including the working 1.3:
And red errors under the console tab:
part of the probem must surely be the "null" origin. But this CORS check seems to be a problem. I hope there is an easier solution than this, but this seems to be pertinent: https://www.youtube.com/watch?v=PNtFSVU-YTI |
CORS is a browser thing, they don't like running code from files directly. Open a command prompt and |
So if he is getting a CORS issue most likely it's how the js page is loaded. I will need to check it and push some changes to fix it. I will be looking myself soon |
Interesting not sure why Chrome would be blocking from loading a local webpage like it is. It really sucks to have to serve this via a web server I will say i can replicate the issue. I honestly think its time we move to VueJS. Dont get me wrong React and Angular are good but the issue we have is learning curve. Vue is very easy for anyone to learn and using TS for the project so everythign comes out with proper variable types it should be simple enough for anyone to understand if they know how to read and write code. If you are familiar enough with JS it will work just the same as we currently have it. I will try to see what i can put together over the next week as im not working. Vue 3 is so powerful and easy to use i usually can write a basic website within a few hours with pretty complex page setup. |
It's a security thing, if a user downloaded a random html page and tried to run it locally without CORS it could potentially read local files and send them to a remote server. It's a bit of a pain but I'd imagine anyone working on this would be using VS Code so we'd have access to Python. A I must have a look at Vue.js as my throttle has a web interface. |
Yeah if you just really nice I'm going to try to put up a project here shortly and give some access to it so people can see it. The good news is with Vue I should be able to take the current web throughout all we have and just convert it mostly without having to change much code. Like I said I'll work on it this weekend and hopefully we get something up and you guys can take a look and see if you like it. |
But what's different now? Why does the old version work? The only reason for having this was that you could just unzip files to a folder and it would run. If there is any more hassle to make it run than that,it is useless. Might as well just write an EXE program. |
Exactly which is why i am moving to attempt writting in Vue as it gives all the files needed in one package and is easy to use hopefully it wont do the CORS issue i will be testing |
Looking at the errors again and the changed files the |
So i have posted a new repo if you guys would like to take a look its using Vue 3. I have not finished the full refactor and gotten the code back to what it currently is but i will be working on it over the next week or so. I hope to have it done soon. At this time only the basics are setup from each page to starting the throttle page refactor. Good news is most of the work already complete will transfer over but there will be some caveats as moving to vue removes all the $ jquery stuff and will allow us direct control over each button and ability to become very reactive. If this does not look like something we would want to commit to please let me know. This is just a test and a start of trying to move to better more sustainable framework than jQuery. |
I finally got round to trying out my React-based throttle on my own layout - a mostly positive experience 👌. I have to say the state management is the biggest challenge but I have managed to keep it documented, but messy |
Very interesting to be sure ya state management in react is very challenging. I haven't added state management yet to the Vue project that will be done today. I will say managing state in Vue is as simple as this.$store.commit('locos/add', {loco:'blah'}); to start a state change once u setup a proper store with mutations and what not. My issue will be the actual serial aspect and getting the proper commands built but I think we have that code setup and able to port it easily. |
So just added state management via Pinia which appears to be easier than using Vuex and is the successor to it. @dcyoung-dev I am curious if you are in the Discord so we can discuss things in a much faster fashion and be able to work together to come up with a solution. Here is a link in case you are not yet. |
I have put together a Command Library and a Serial communicator library that you could use in your project. I am on Discord but I rarely use it - my phone is mostly on silent so replies will probably be just as slow. |
What CSS framework are you using for your cloudthrottle I am looking at this and wanting to move away from the fully custom and jquery-ui that we are currently using. I normally use Vuetify with Vue as its simple elegant and very very easy to use. I should be able to recreate the entire throttle setup from most of what we have already but some things will not be exactly the same. I think it will work out better especially since i am now thinking of incorporating the installer into WebThrottle which is what @FrightRisk wanted to do to begin with. This Arduino Create agent may be able to be used to my advantage for this along with making it easier for every one to use the isntaller as web browsers are truly cross platform already. It will also allow the WebThrottle to be more mobile friendly as it automatically works and modifies page properly for mobile devices. I like the current setup but it is unrealistic in the long run to write out all the code needed to be able to do everything that jquery does along with making it compatible with Vue or React or what ever we decide to use. |
I've used the Simple CSS framework. I've not made much custom CSS other than the Throttle page but it's all compositional CSS that I've added. |
Stale |
What?
I have added/removed/altered:
Why?
I am doing this because:
Acceptance Criteria
I can call this done when:
Deployment risks
dcc-ex--commands-0.4.0.js
will become outdated as project changesCloses #53