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

Refactor function buttons #71

Closed

Conversation

dcyoung-dev
Copy link

@dcyoung-dev dcyoung-dev commented Dec 17, 2021

What?

I have added/removed/altered:

  • Moved FunctionButton functions to separate file
  • Created raw command writer function
  • Removed Legacy function command format
  • Removes/refactors jQuery implementation
  • Adds command library
  • Rearranges JS dependancies

Why?

I am doing this because:

  • Previous Function Button logic generated legacy commands
  • Uses modularised JS to improve code separation
  • New command format can be used
  • Using the command library gives access to all documented commands

Acceptance Criteria

I can call this done when:

  • Function Buttons send new format commands

Deployment risks

  • Will no longer send legacy command format
  • I'm happy to discuss any of these changes.
  • dcc-ex--commands-0.4.0.js will become outdated as project changes

Closes #53

@FrightRisk
Copy link
Member

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?

@dcyoung-dev
Copy link
Author

@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.

@FrightRisk
Copy link
Member

FrightRisk commented Feb 27, 2022 via email

@dcyoung-dev
Copy link
Author

@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 🙃

@FrightRisk
Copy link
Member

FrightRisk commented Mar 12, 2022 via email

@ManiAkasapu
Copy link
Member

@dcyoung-dev It is good that some real developer looking at this. Thank you for contributing.

@FrightRisk
Copy link
Member

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?

Copy link
Member

@dexslab dexslab left a 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

@FrightRisk
Copy link
Member

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.

@FrightRisk
Copy link
Member

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

@dcyoung-dev dcyoung-dev force-pushed the refactor-function-buttons branch from 556ff19 to 296b9e7 Compare May 8, 2022 20:32
@dcyoung-dev
Copy link
Author

dcyoung-dev commented May 8, 2022

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

Should all be fixed in 296b9e7

Next question is if we need to move to Vue.js

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?

@FrightRisk
Copy link
Member

I am open to any tool that does the job and can be documented so if someone leaves the project, others can follow

@FrightRisk
Copy link
Member

FrightRisk commented May 10, 2022

Still doesn't work. I go to your repo, download the zip, unzip it to a folder and run index.

  • It looks like no commands are being sent.
  • Nothing shows up in the debug log other than incoming messages from the CS.
  • I can click on the "connect" button, but clicking the "disconnect" button does nothing. It stays showing the disconnect text.
  • I can't manually send a command. Entering "1" to turn on the power does nothing and nothing shows in the log.
  • None of the buttons depress or send commands.

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.

@dexslab
Copy link
Member

dexslab commented May 11, 2022

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.

@dcyoung-dev
Copy link
Author

I'm not sure what's going on @FrightRisk as I do not see the same behaviour. ☹️
https://user-images.githubusercontent.com/8854356/168155230-9c09e9f5-53e9-4949-995b-dfe2234bbbde.mp4

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

@alex-code
Copy link

@FrightRisk which browser are you using?
Do you get any errors if you open the browser console?

@FrightRisk
Copy link
Member

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.

@FrightRisk
Copy link
Member

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.

@FrightRisk
Copy link
Member

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.

@FrightRisk
Copy link
Member

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.

@dcyoung-dev
Copy link
Author

@FrightRisk it sounds like there are a lot of issues to sort through.

Have you tried downloading the master branch from the DCC-EX repo and checking that it works? If it does and my branch isn't working then we can look into it. If the master also isn't working then there's a bigger problem.

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

This is the browser's developer tool which you can get to by right-clicking anywhere in the UI and selecting inspect.

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

@dexslab
Copy link
Member

dexslab commented May 13, 2022

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

@FrightRisk
Copy link
Member

FrightRisk commented May 13, 2022

Issue

I believe my issue is because I run from my folder and not from a web server.

Testing 1.3 released version

Works perfectly despite 2 errors showing up in the console log as shown below in Master that has the same 2 errors.

Testing Master

Master 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:

Uncaught (in promise) TypeError: Failed to register a ServiceWorker: The URL protocol of the current 
origin ('null') is not supported. at index.html:492:31

and this:

Access to internal resource at 'file:///C:/Users/Fred/Downloads/WebThrottle-EX-master/manifest.json' from 
origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol 
schemes: http, data, chrome, chrome-extension, chrome-untrusted, https.

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 version

I see this under the "issues" tab in the inspector. Though this also appears on all versions, including the working 1.3:

A page or script is accessing at least one of navigator.userAgent, navigator.appVersion, and navigator.platform. 
Starting in Chrome 101, the amount of information available in the User Agent string will be reduced.
To fix this issue, replace the usage of navigator.userAgent, navigator.appVersion, and navigator.platform 
with feature detection, progressive enhancement, or migrate to navigator.userAgentData. 
Note that for performance reasons, only the first access to one of the properties is shown.

And red errors under the console tab:

Access to script at 'file:///C:/Users/Fred/Downloads/WebThrottle-EX-refactor-function-buttons/js/ui/functionButtons.js' 
from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol 
schemes: http, data, chrome, chrome-extension, chrome-untrusted, https.
functionButtons.js:1          

Failed to load resource: net::ERR_FAILED
index.html:492 

Uncaught (in promise) TypeError: Failed to register a ServiceWorker: The URL protocol of the current 
origin ('null')  is not supported. at index.html:492:31

Access to internal resource at 'file:///C:/Users/Fred/Downloads/WebThrottle-EX-refactor-function-buttons/manifest.json' 
from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol 
schemes: http, data, chrome, chrome-extension, chrome-untrusted, https.

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

@alex-code
Copy link

CORS is a browser thing, they don't like running code from files directly.
Have you got Python installed? If so you can fire up a simple webserver.

Open a command prompt and cd to the directory with the code then run python3 -m http.server.
You can then access it via http://127.0.0.1:8080/

@dexslab
Copy link
Member

dexslab commented May 14, 2022

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

@dexslab
Copy link
Member

dexslab commented May 14, 2022

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.

@alex-code
Copy link

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 bat file could be included to make it easier to load up Python as a server.

I must have a look at Vue.js as my throttle has a web interface.

@dexslab
Copy link
Member

dexslab commented May 14, 2022

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 bat file could be included to make it easier to load up Python as a server.

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.

@FrightRisk
Copy link
Member

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.

@dexslab
Copy link
Member

dexslab commented May 15, 2022

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

@alex-code
Copy link

Looking at the errors again and the changed files the functionButtons.js has a module type which doesn't work locally.
Is the ServiceWorker needed if it's intended to be run locally? If we're local there's no need to cache.

@dexslab
Copy link
Member

dexslab commented May 16, 2022

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.

https://github.com/DCC-EX/WebThrottleEx-Vue

@dcyoung-dev
Copy link
Author

I finally got round to trying out my React-based throttle on my own layout - a mostly positive experience 👌.
https://www.cloudthrottle.app/

I have to say the state management is the biggest challenge but I have managed to keep it documented, but messy

@dexslab
Copy link
Member

dexslab commented May 16, 2022

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.

@dexslab
Copy link
Member

dexslab commented May 16, 2022

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.

https://discord.com/invite/y2sB4Fp

@dcyoung-dev
Copy link
Author

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.

@dexslab
Copy link
Member

dexslab commented May 16, 2022

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.

@dcyoung-dev
Copy link
Author

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.

@dcyoung-dev
Copy link
Author

Stale

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.

Change (add?) <F> command for handling function buttons
5 participants