arduino / Arduino

Arduino IDE 1.x
https://www.arduino.cc/en/software
Other
14.13k stars 7k forks source link

[IDE] Slow Startup - Create Examples menus for Libraries (need ignore files) #10235

Open ricardojlrufino opened 4 years ago

ricardojlrufino commented 4 years ago

Hi, I realized that also the code that builds the examples of the libraries, may have a problem. If the library has large documentation (docs) like generated by doxygen, or some other directory with many files, the IDE will take a long time to scan.

My first idea was to ignore the known files ... as is done with .svn and .git ... but I ended up identifying that it is better to do this treatment below:

https://github.com/arduino/Arduino/blob/02a2e22649feb4d85d807b51122f2501b06da866/app/src/processing/app/Base.java#L1779-L1781

to this: To only focus on the folder of interest.

private boolean addSketchesSubmenu(JMenu menu, UserLibrary lib) {
    File examples = new File(lib.getInstalledFolder(), "examples");

    JMenu submenu = new JMenu(lib.getName());
    boolean found = addSketches(submenu, examples);
    if (found) {
      menu.add(submenu);
      MenuScroller.setScrollerFor(submenu);
    }

    return found;
  }

In my case I had a node_modules folder (don't ask me why ... lol) it took about 5s to read it total time was around 11s, because i had another library with a big docs folder.

PS: Sorry for opening several separate problems, but it is for us to track the errors I found. As it is part of an ongoing refactoring, in order to solve part of the same "slow startup" problem, it is difficult to make several separate pull requests.

matthijskooijman commented 4 years ago

I had to look at the code a bit closer, but it seems there's two versions of addSketchesSubmenu, where the three-argument versions is used to build the regular sketches menu (recursively scanning the sketchbook folder to build a nested menu) and the "base examples" part of the examples menu (through addSketches), and the two-argument version is called for each library to fill the examples menu for that library.

Then, the two-argument library version then calls the three-argument version (as shown in your post). This then recursively scans the entire library dir for sketches, rather than just the "examples" directory. You would wonder why this does not show up submenus labeled "examples" everywhere, but that is because there is a special case here: https://github.com/arduino/Arduino/blob/02a2e22649feb4d85d807b51122f2501b06da866/app/src/processing/app/Base.java#L1833-L1835

I actually think that this might be a bug in itself: If a library includes sketches outside of the examples directory, those are also show in the examples menu. I just tested this by adding an empty sketch to just some library in my sketchbook:

libraries/AccelStepper$ find foo/
foo/
foo/bar
foo/bar/bar.ino

image

Your proposal seems to also fix this, which I think is a good thing.

However, your proposal seems to duplicate some code, the code you added is already here: https://github.com/arduino/Arduino/blob/02a2e22649feb4d85d807b51122f2501b06da866/app/src/processing/app/Base.java#L1838-L1844

I thought it could be reused by just passing the examples library subdir to the three-argument version of addSketchesSubmenu, but that would mean all libraries show up as "examples" submenus instead of the library name (and also, if there is an examples/examples.ino that would show up without a submenu at all).

Maybe a little refactoring could help here, to have a function that says "Add a submenu with this name containing the subdirectories of this directory recursively" helper function could be added to be used in both cases?

PS: Sorry for opening several separate problems

Probably a good idea, since it allows separating discussions about each problem easily, and referring to each problem separately in commit messages.

ricardojlrufino commented 4 years ago

I thought it could be reused by just passing the examples library subdir to the three-argument version of addSketchesSubmenu, but that would mean all libraries show up as "examples" submenus instead of the library name (and also, if there is an examples/examples.ino that would show up without a submenu at all).

Yes, it was the first test I did. And I realized this problem.

Maybe a little refactoring could help here, to have a function that says "Add a submenu with this name containing the subdirectories of this directory recursively" helper function could be added to be used in both cases?

It already exists, as you mentioned: addSketchesSubmenu (JMenu menu, String name, File folder), I think creating another would be the most confusing. You already have many methods and overheads for create menus.

The fix I made adds little code and reuses existing functions

cmaglie commented 4 years ago

Another problem is that not all libraries put the examples in the examples folder, many uses different combination of case and plural/singular: Example, example, EXAMPLES, I think we should handle any combination of them.

matthijskooijman commented 4 years ago

Should we really cater for those? The library spec clearly says it should be "examples" in lowercase, and the fact that those work at all is basically due to a bug I think (and even if they do work, you'll have an extra "Example" submenu now).

I would be in favor of only supporting examples and letting the libraries fix this. Maybe case insensitive matching would be ok, but also more tricky to implement, I guess.

Would it be hard to scan all libraries in the library manager for any folders that look like examples but are not exactly? Maybe even just file bugs against?

cmaglie commented 4 years ago

Should we really cater for those? The library spec clearly says it should be "examples" in lowercase, and the fact that those work at all is basically due to a bug I think (and even if they do work, you'll have an extra "Example" submenu now).

That's the problem, people will see the examples disappear on some libraries with no apparent reason -> more issues to handle later.

Would it be hard to scan all libraries in the library manager for any folders that look like examples but are not exactly? Maybe even just file bugs against?

Let me check this...

cmaglie commented 4 years ago

This is a qucik&dirty list I've extracted from the library indexer, it may be incomplete or inaccurate in some extreme cases...

Folder Library
example_arduino https://github.com/hideakitai/CRC/tree/master/example_arduino
example_arduino https://github.com/hideakitai/CRCx/tree/master/example_arduino
example_cpp https://github.com/hideakitai/CRC/tree/master/example_cpp
example_cpp https://github.com/hideakitai/CRCx/tree/master/example_cpp
Example_DFPlayerFast_Sketch https://github.com/PowerBroker2/DFPlayerMini_Fast/tree/master/Example_DFPlayerFast_Sketch
example https://github.com/andhieSetyabudi/atlas_OEM/tree/master/example
example https://github.com/andhieSetyabudi/BQ25896/tree/master/example
example https://github.com/arash77/BaleMessengerBot_Arduino/tree/master/example
example https://github.com/budryerson/TFMini-Plus-I2C/tree/master/example
example https://github.com/budryerson/TFMini-Plus/tree/master/example
example https://github.com/datacentricdesign/dcd-sdk-arduino/tree/master/example
example https://github.com/FaBoPlatform/FaBoGPIO40-PCA9698-Library/tree/master/example
example https://github.com/FaBoPlatform/FaBoPWM-PCA9685-Library/tree/master/example
example https://github.com/foothillscommunityworkshop/Robot-Model-2/tree/master/example
example https://github.com/GreenPonik/DFRobot_ESP_EC_BY_GREENPONIK/tree/master/example
example https://github.com/GreenPonik/DFRobot_ESP_PH_WITH_ADC_BY_GREENPONIK/tree/master/example
example https://github.com/hideakitai/ArxSmartPtr/tree/master/example
example https://github.com/hideakitai/Convert/tree/master/example
example https://github.com/hideakitai/Filters/tree/master/example
example https://github.com/hideakitai/I2CExtension/tree/master/example
example https://github.com/hideakitai/MAX17048/tree/master/example
example https://github.com/hideakitai/MCP4728/tree/master/example
example https://github.com/hideakitai/MTCParser/tree/master/example
example https://github.com/hideakitai/PCA9536/tree/master/example
example https://github.com/hideakitai/PCA9547/tree/master/example
example https://github.com/hideakitai/PCF2129/tree/master/example
example https://github.com/hideakitai/SpresenseNeoPixel/tree/master/example
example https://github.com/hideakitai/TCS34725/tree/master/example
example https://github.com/hideakitai/TFmini/tree/master/example
example https://github.com/hideakitai/VectorXf/tree/master/example
example https://github.com/hideakitai/XBeeATCmds/tree/master/example
example https://github.com/i3water/Blinker_PMSX003ST/tree/master/example
example https://github.com/MAINAKMONDAL98/MSMPLOTTER/tree/master/example
example https://github.com/mertwhocodes/mwc_stepper/tree/master/example
example https://github.com/MikeDombo/HV518_Arduino/tree/master/example
example https://github.com/neonode-inc/zforce-arduino/tree/master/example
example https://github.com/Protocentral/protocentral_healthypi4_arduino/tree/master/example
example https://github.com/remoteme/esp8266-OLED/tree/master/example
example https://github.com/Seeed-Studio/Grove_BMP280/tree/master/example
example https://github.com/Seeed-Studio/Seeeduino_GPRS/tree/master/example
example https://github.com/SohnyBohny/6-digit-7-Segment-Arduino/tree/master/example
example https://github.com/thesolarnomad/lora-serialization/tree/master/example
example https://github.com/TridentTD/TridentTD_EasyFreeRTOS32/tree/master/example
example https://gitlab.com/robostarter/starterremote/tree/master/example
example-PCA9546A https://github.com/hideakitai/PCA9547/tree/master/example-PCA9546A
Examples https://github.com/4dsystems/GFX4DIoD9/tree/master/Examples
Examples https://github.com/4dsystems/GFX4d/tree/master/Examples
Examples https://github.com/Annikken/Andee101/tree/master/Examples
Examples https://github.com/avandalen/avdweb_Switch/tree/master/Examples
Examples https://github.com/dingusdk/arduinoihc/tree/master/Examples
Examples https://github.com/dniklaus/wiring-timer/tree/master/Examples
Examples https://github.com/jackrobotics/iSYNC_BC95_Arduino/tree/master/Examples
Examples https://github.com/janthefischer/SerialVariable/tree/master/Examples
Examples https://github.com/JChristensen/JC_Button/tree/master/Examples
Examples https://github.com/JChristensen/MCP79412RTC/tree/master/Examples
Examples https://github.com/JChristensen/movingAvg/tree/master/Examples
Examples https://github.com/JChristensen/Timezone/tree/master/Examples
Examples https://github.com/JemRF/max7219/tree/master/Examples
Examples https://github.com/jmderomedi/SavitzkyGolayFilter/tree/master/Examples
Examples https://github.com/JoaoLopesF/SerialDebug/tree/master/Examples
Examples https://github.com/kosme/arduinoFFT/tree/master/Examples
Examples https://github.com/kosme/timestamp32bits/tree/master/Examples
Examples https://github.com/LowPowerLab/RFM69/tree/master/Examples
Examples https://github.com/muwerk/munet/tree/master/Examples
Examples https://github.com/muwerk/muwerk/tree/master/Examples
Examples https://github.com/muwerk/ustd/tree/master/Examples
Examples https://github.com/netpieio/microgear-nbiot-arduino/tree/master/Examples
Examples https://github.com/ReefPOM/OSPOM/tree/master/Examples
Examples https://github.com/RobTillaart/FastShiftOut/tree/master/Examples
Examples https://github.com/rocketscream/Low-Power/tree/master/Examples
Examples https://github.com/Rom3oDelta7/LED3/tree/master/Examples
Examples https://github.com/Seeed-Studio/Grove_IMU_9DOF/tree/master/Examples
Examples https://github.com/shaduzlabs/arduino-rastr/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Ambient_Light_Sensor_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Ambient_Light_Sensor_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Ambient_Light_Sensor_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Ambient_Light_Sensor_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Ambient_Light_Sensor_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_APDS9301_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Color_LCD_Shield_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/sparkfun/SparkFun_Qwiic_Relay_Arduino_Library/tree/master/Examples
Examples https://github.com/Tdoe4321/FlexLibrary/tree/master/Examples
Examples https://github.com/uStepper/egoShieldS/tree/master/Examples
Examples https://github.com/uStepper/egoShieldTeach/tree/master/Examples
Examples https://github.com/uStepper/egoShieldTimeLapse/tree/master/Examples
Examples https://github.com/uStepper/uStepper-S-lite/tree/master/Examples
Examples https://github.com/uStepper/uStepperS/tree/master/Examples
Examples https://github.com/uStepper/uStepper/tree/master/Examples
extras https://github.com/fhessel/esp32_https_server_compat/tree/master/extras
extras https://github.com/fhessel/esp32_https_server/tree/master/extras
extras https://github.com/rleddy/tinycmdtable/tree/master/extras
https://github.com/GordonRudman/ExampleArduinoLibrary/tree/master/
libraries https://github.com/Gamebuino/Gamebuino-Classic/tree/master/libraries
res https://github.com/FortySevenEffects/arduino_midi_library/tree/master/res
cmaglie commented 4 years ago

Looking at the list again, just adding Examples and example will cover 99% of the cases and the remaining may be fixed upstream. If we do that we should fix the libraries specification accordingly.

per1234 commented 4 years ago

In principle, I like having a single specific folder name for examples, but the fact that the IDE has recognized any sketch as an example for so long means there are a lot of libraries that don't follow the specification. I think being a little fuzzy with the supported folder name, but restricting support to variants of "examples" that clearly indicate the library author intended this folder to contain examples is a good compromise.

This would fix https://github.com/arduino/Arduino/issues/5688 and https://github.com/arduino/Arduino/issues/7315.


One thing to consider when looking at the list above is that quite a few of those example folders contain a .ino file directly under it that doesn't match the folder name:

SomeLibrary
|_ example
   |_ Foo.ino

and so wouldn't be recognized by the Arduino IDE anyway. There are also some where the example folder is the sketch itself, rather than being a folder that contains sketches:

SomeLibrary
|_ example
   |_ example.ino

Would it be hard to scan all libraries in the library manager for any folders that look like examples but are not exactly? Maybe even just file bugs against?

I have a script that scans all the Arduino libraries on GitHub for common problems, including containing sketches that are outside a specification-compliant folder. I have made some efforts to submit PRs to fix this on my campaigns to fix the issues reported by the scan, but I found it difficult to convince some library authors that it was worth making the change to follow the specification when the IDE didn't enforce the specification, so I didn't consider that particular issue to be a priority for my efforts.

With a clear signal that this will no longer be supported in the next release, I think I would have much more success. I'd be willing to do a run of submitting PRs to the libraries that are using unsupported sketch locations in order to mitigate the potential impact of this change.

ricardojlrufino commented 4 years ago

I tested some layouts, and we have the following result

ALIB_Test01  *** MATCH ***
├── examples
│   └── ALIB_Test01_example
│       └── ALIB_Test01_example.ino
└── src
ALIB_Test02
├── examples
│   ├── example1.ino
│   └── example2.ino
└── src
ALIB_Test03 *** MATCH ***
├── Example
│   └── Lib03_Demo1
│       └── Lib03_Demo1.ino
└── src
ALIB_Test04
├── example_arduino
│   └── example_arduino.ino
├── example_esp8266
│   └── example_esp8266.ino
└── src
ALIB_Test05 *** MATCH ***
├── Examples
│   └── Working51
│       └── Working51.ino
└── src

Supporting ALIB_Test02, may be good..

Implementation:

  private boolean addSketchesSubmenu(JMenu menu, UserLibrary lib) {

    JMenu submenu = new JMenu(lib.getName());
    boolean found = false;

    // Compatibility mode: not all community libraries are following the specification, look for common names found.
    File[] list = lib.getInstalledFolder().listFiles(new ExamplesFilter());
    for (File folder : list) {
      if(addSketches(submenu, folder)) {
        found = true;
      }
    }

    if (found) {
      menu.add(submenu);
      MenuScroller.setScrollerFor(submenu);
    }

    return found;
  }
public class ExamplesFilter extends OnlyDirs {

    @Override
    public boolean accept( File dir , String name ) {

        if(!super.accept(dir, name)) return false;

        return name.toLowerCase().startsWith("example") ;
    }

}
matthijskooijman commented 4 years ago
ALIB_Test02
├── examples
│   ├── example1.ino
│   └── example2.ino

Supporting ALIB_Test02, may be good..

I don't think so. Those are never valid sketches, so it will require significant code to allow loading them, and then users could be very much confused that these sketches work like this as an example, but break when they copy them elsewhere (e.g. without a top-level directory that matches the .ino name).

Your code proposal looks good, using such a filter neatly takes care of all the different ways and casings. Also, with this additional filtering, the code duplication I mentioned before is reduced (because this now includes potentially multiple directories in the same menu) and probably not a problem anymore.

I do wonder: Do we need that found variable? I think you could also just see if menu is still empty or not? This same I think applies to the existing code as well, might be a nice and small simplification to include in the same PR (different commit, of course).

cmaglie commented 4 years ago

IIRC the found is needed to get rid of nested "empty" trees, otherwise, very/long/empty/path/ will be added as a menu (very->long->empty->). But I may be wrong :-)

@ricardojlrufino

I would change the condition

startsWith("example")

with

equalsIgnoreCase("example") || equalsIgnoreCase("examples") 

othewise also exampleXXXXXX will match. Another thing that I would change is to break the searching loop after the first match otherwise it may match many times for example in the following case:

ALIB_Test06
├── examples           <- MATCH here
│   └── Blink
│       └── Blink.ino
├── Example            <- and here too!
│   └── Blink          <- same Blink sketch name
│       └── Blink.ino
└── src

for the rest look good to me.

matthijskooijman commented 4 years ago

IIRC the found is needed to get rid of nested "empty" trees, otherwise, very/long/empty/path/ will be added as a menu (very->long->empty->). But I may be wrong :-)

If each recursive call takes care to never add a menu unless it is non-empty, I think it should be ok?

Agreed with your other comments, @cmaglie.

cmaglie commented 4 years ago

If each recursive call takes care to never add a menu unless it is non-empty, I think it should be ok?

I think it will get more complicated than just returining found but I've not much mind-time to figure out, so I'll just leave it to the implementer :-)

matthijskooijman commented 4 years ago

Looking at the list again, just adding Examples and example will cover 99% of the cases and the remaining may be fixed upstream. If we do that we should fix the libraries specification accordingly.

If we update the spec, probably also include there that the examples folder(s) are scanned recursively, so example sketches can be categorized by putting them in subfolders (this is currently the case already, but not made explicit in the spec).

https://github.com/arduino/arduino-cli/blob/master/docs/library-specification.md#library-examples

ricardojlrufino commented 4 years ago

@cmaglie

equalsIgnoreCase("example") || equalsIgnoreCase("examples")

will not match that cases.

image

ricardojlrufino commented 4 years ago

My idea was to support this format too, which is similar to the print above.

ALIB_Test06 ├── Examples_AVR │   └── DemoAVR │   └── DemoAVR.ino ├── Examples_ESP8266 │   └── DemoEsp │   └── DemoEsp.ino └── src

per1234 commented 4 years ago

will not match that cases.

IMO, that's fine. They will just need to move the examples to a specification-compliant location. Only the example_arduino ones are even Arduino sketches.

It is unfortunate that the user experience will be worsened by the examples of some unmaintained libraries no longer being visible, but the current behavior of picking up all sketches is also causing some harm, so I think it's worth the trade-off.

per1234 commented 4 years ago

My idea was to support this format too

I really doubt that's a common convention. It doesn't seem worth adding extra complexity to the code to support this very rare usage.

ricardojlrufino commented 4 years ago

I really doubt that's a common convention. It doesn't seem worth adding extra complexity to the code to support this very rare usage.

Ok, I agree to align the specification: Then we will have:

OK ?!

ricardojlrufino commented 4 years ago

Now let's move on to the next problem, similar to this one, which is related to the 'Skecthbook' folder.

In my case I have a Sketch that has the node_modules folder, and it takes a long time to scan. I can imagine now only two options ...

sgparry commented 4 years ago

re: libraries with examples folders not called 'examples': The library standards documentation clearly says examples live in a folder called examples. Does nobody read????. By supporting loads of other cases, this is very confusing for everyone. I just got bitten by this issue - I found a while back that by adding an empty src.ino file to my libraries src folder, I could workaround the IDEs reluctance to edit libraries created in 1.5+ library format. Unfortunately I now have example menus littered with spurious entries called src. I think handling case insensitively and accepting singular is OK - but I would just stop there.