Fix bug when cloning a story with windows #643

macumber · 2023-11-23T06:06:49Z

Includes build output of openstudiocoalition/floorspace.js#1

Notice I had to disable the Uglify plugin in the build process here:

https://github.com/openstudiocoalition/floorspace.js/pull/1/files#diff-428f295cce6ac12808d1771cf1dcada74b6dd5c38ac92555ac07e6a2fc28a8b4R34

I was hoping NREL might be able to update the floorspace.js build scripts but sounds like that is not in their current plan

jmarrec · 2023-11-24T11:49:47Z

@macumber I can't even see the diff because it's too large.

Could we separate the jS like I did on Geometry Preview refactor - separate into several files, allow running in browser directly #628 ?
Can we figure out a way to uglify it anyways, even if that means manually editing the bit that is faulty?

…_clone_w_windows

macumber · 2023-11-25T04:07:31Z

I was hoping NREL would be able to fix the build scripts, but now that we know that is not happening in the near future, I can try getting uglify to work again.

I can try getting the floorspace.js build scripts to separate the JavaScript from html, but I'm not an expert in those build scripts

macumber · 2023-12-09T20:52:46Z

@jmarrec I'm trying to look into this again, I put out a request on Fiverr to see if someone can update webpack from version 1 to version 4 or 5, I think that is what is needed to get the uglify working again (now it is just an option in webpack instead of a plugin).

I don't quite understand your request about breaking the JavaScript out. You want to separate the html from the JavaScript? There is a build-embeddable.js script that makes the embeddable_geometry_editor.html (adds API for the OS App to use) and standalone_geometry_editor.html (can be opened in a local browser) files. There really isn't much interesting html, it's almost all JavaScript.

jmarrec · 2023-12-11T09:47:21Z

@macumber Yeah I was just wondering if we could have the HTML reference a .js file, so we could see the HTML structure a bit better. And we could potentially just locally change the minified with the full one during testing/debugging when needed. Something like

-<script src="js/floorspace.min.js"></script>
+<script src="js/floorspace.js"></script>

That'd also allow to differentiate between external libs and our own code/wrappers around these libs.

I've found that helpful for the geometry_preview.html. But I'm not forcing anything here.

jmarrec · 2023-12-11T09:48:29Z

@macumber In the interim, I'm perfectly happy if you want to merge this one and we open a subsequent one later

macumber · 2023-12-18T03:50:09Z

I've updated this PR with the new minified version of FloorspaceJS built from https://github.com/openstudiocoalition/floorspace.js

Initial testing looks good, I'll try using it from an install package here

macumber · 2023-12-18T03:57:09Z

The html for the embedded editor that is separate from the built JavaScript is just hard coded at: https://github.com/openstudiocoalition/floorspace.js/blob/develop/build/build-embeddable.js#L19

macumber@Automatic-Magic-PC MINGW64 /c/openstudioapplication-1.7.0-rc0/bin $ ./openstudio classic measure -s 1616 [2023-12-18 20:53:58] INFO WEBrick 1.6.0 [2023-12-18 20:53:58] INFO ruby 2.7.2 (2020-10-01) [x64-mswin64_140] [2023-12-18 20:53:58] INFO WEBrick::HTTPServer#start: pid=17592 port=1616 127.0.0.1 - - [18/Dec/2023:20:54:15 Mountain Standard Time] "GET / HTTP/1.1" 200 79 - -> / [2023-12-18 20:54:35] INFO going to shutdown ... [2023-12-18 20:54:35] INFO WEBrick::HTTPServer#start done. ┌────────────────────────────────────────────────────────────────────────────────┐ │ The `classic` command is deprecated and will be removed in a future release │ └────────────────────────────────────────────────────────────────────────────────┘ macumber@Automatic-Magic-PC MINGW64 /c/openstudioapplication-1.7.0-rc0/bin $ ./openstudio measure -s 1616 MeasureManager ReadyAccepting requests on: http://localhost:1616/

macumber · 2023-12-19T04:01:11Z

src/openstudio_app/OpenStudioApp.cpp

@@ -1288,7 +1288,7 @@ void OpenStudioApp::startMeasureManagerProcess() {
  tcpServer.close();

  QString portString = QString::number(port);
-  QString urlString = "http://127.0.0.1:" + portString;
+  QString urlString = "http://localhost:" + portString;


@jmarrec what do you think about this? For some reason when I run the app from the installer it can't find the measure manager. Is there some reason the new measure manager is only listening to localhost and not 127.0.0.1? I always thought the two were the same for IPv4.

macumber@Automatic-Magic-PC MINGW64 /c/openstudioapplication-1.7.0-rc0/bin $ ./openstudio classic measure -s 1616 [2023-12-18 20:53:58] INFO WEBrick 1.6.0 [2023-12-18 20:53:58] INFO ruby 2.7.2 (2020-10-01) [x64-mswin64_140] [2023-12-18 20:53:58] INFO WEBrick::HTTPServer#start: pid=17592 port=1616 127.0.0.1 - - [18/Dec/2023:20:54:15 Mountain Standard Time] "GET / HTTP/1.1" 200 79 - -> / [2023-12-18 20:54:35] INFO going to shutdown ... [2023-12-18 20:54:35] INFO WEBrick::HTTPServer#start done. ┌────────────────────────────────────────────────────────────────────────────────┐ │ The `classic` command is deprecated and will be removed in a future release │ └────────────────────────────────────────────────────────────────────────────────┘ macumber@Automatic-Magic-PC MINGW64 /c/openstudioapplication-1.7.0-rc0/bin $ ./openstudio measure -s 1616 MeasureManager ReadyAccepting requests on: http://localhost:1616/

You're on windows I assume (edit: I see MINGW64, so yes)? I kinda just gave up trying to make both localhost and 127.0.0.0 work on windows... It's a limitation of the cpprestsdk afaik. https://github.com/NREL/OpenStudio/blob/05c60768c1e1057083ce644d49711f7377f8ce1e/src/cli/MeasureManager.cpp#L563-L569

macumber · 2023-12-23T05:31:12Z

@jmarrec I'm planning to merge this and make an RC1 tomorrow, sound good? I was going to try upgrading to Qt 6.6 as well but there is a bug in moc on Mac

macumber · 2023-12-23T05:32:51Z

src/shared_gui_components/MeasureManager.cpp

@@ -579,6 +579,10 @@ boost::optional<measure::OSArgument> MeasureManager::getArgument(const measure::
    Json::Value choiceValues = argument.get("choice_values", Json::Value(Json::arrayValue));
    Json::Value choiceDisplayNames = argument.get("choice_display_names", Json::Value(Json::arrayValue));

+    if (choiceValues.empty()) {
+      choiceValues = argument.get("choices_values", Json::Value(Json::arrayValue));


This is a bug in the OS repo, there is a typo of choices_values instead of choice_values

I fixed it already in NREL/OpenStudio#5046 but it wasn't selected for 3.7.0 official... I specifically asked for it to be included but it was too last minute

jmarrec · 2023-12-23T07:28:57Z

@jmarrec I'm planning to merge this and make an RC1 tomorrow, sound good? I was going to try upgrading to Qt 6.6 as well but there is a bug in moc on Mac

Sounds good. I'll go check #650

jmarrec · 2023-12-23T08:01:58Z

Actually I just realized I never made a PR for the Python measures branch I had going on...

Fix bug when cloning a story with windows

bb90b54

Merge remote-tracking branch 'remotes/origin/develop' into floorspace…

a8a1afd

…_clone_w_windows

Adding minified output from updated build scripts

2be851c

macumber added 2 commits December 18, 2023 17:43

Try to fix macos-12 build

f5ace69

macumber commented Dec 19, 2023

View reviewed changes

macumber added 5 commits December 21, 2023 10:50

Try the classic command

fcbf16b

Seems the new measure manager is sensitive to the content type header

1b845aa

This seems to be working for the measure manager

ca4d511

Update version of FloorspaceJS and add missing data to import

9c7b27b

Fix clang format

e5679e7

macumber requested a review from jmarrec December 23, 2023 05:31

macumber commented Dec 23, 2023

View reviewed changes

jmarrec merged commit b08e3c6 into develop Dec 23, 2023
8 checks passed

jmarrec deleted the floorspace_clone_w_windows branch December 23, 2023 07:38

github-actions bot locked and limited conversation to collaborators Dec 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug when cloning a story with windows #643

Fix bug when cloning a story with windows #643

macumber commented Nov 23, 2023 •

edited

Loading

jmarrec commented Nov 24, 2023

macumber commented Nov 25, 2023 •

edited

Loading

macumber commented Dec 9, 2023

jmarrec commented Dec 11, 2023 •

edited

Loading

jmarrec commented Dec 11, 2023

macumber commented Dec 18, 2023 •

edited

Loading

macumber commented Dec 18, 2023

macumber Dec 19, 2023

jmarrec Dec 23, 2023 •

edited

Loading

macumber commented Dec 23, 2023

macumber Dec 23, 2023

jmarrec Dec 23, 2023 •

edited

Loading

jmarrec commented Dec 23, 2023

jmarrec commented Dec 23, 2023

Fix bug when cloning a story with windows #643

Fix bug when cloning a story with windows #643

Conversation

macumber commented Nov 23, 2023 • edited Loading

jmarrec commented Nov 24, 2023

macumber commented Nov 25, 2023 • edited Loading

macumber commented Dec 9, 2023

jmarrec commented Dec 11, 2023 • edited Loading

jmarrec commented Dec 11, 2023

macumber commented Dec 18, 2023 • edited Loading

macumber commented Dec 18, 2023

macumber Dec 19, 2023

Choose a reason for hiding this comment

jmarrec Dec 23, 2023 • edited Loading

Choose a reason for hiding this comment

macumber commented Dec 23, 2023

macumber Dec 23, 2023

Choose a reason for hiding this comment

jmarrec Dec 23, 2023 • edited Loading

Choose a reason for hiding this comment

jmarrec commented Dec 23, 2023

jmarrec commented Dec 23, 2023

macumber commented Nov 23, 2023 •

edited

Loading

macumber commented Nov 25, 2023 •

edited

Loading

jmarrec commented Dec 11, 2023 •

edited

Loading

macumber commented Dec 18, 2023 •

edited

Loading

jmarrec Dec 23, 2023 •

edited

Loading

jmarrec Dec 23, 2023 •

edited

Loading