Fix unintended extra short beep on package start

[0x41kravchenko]

While using beeper on servo (PPM) pin I noticed that my board makes 2 short beeps and 1 long beep on startup.
And knowing this On boot: short beep when VESC boots, long beep when balance app is ready (and each time you write app config!) or triple-beep when float package is disabled from Float package article by surfdado (IFAIK beeper code was unchanged in refloat) it made me think that 1 extra short beep is due to some errors/faults.
According to current implementation correct behaviour indeed should be 1 short beep (in configure()) and 1 long beep (when board is ready).
1 short beep (in configure()):

refloat/src/main.c

Line 227 in a86a3eb

beep_alert(d, 1, false);

1 long beep (when board is ready)

refloat/src/main.c

Lines 847 to 848 in a86a3eb

	// Let the rider know that the board is ready (one long beep)
	beep_alert(d, 1, true);

That one extra short beep happens unintentionally due to beeper_pin having high level right after pin init here:

refloat/src/main.c

Line 105 in a86a3eb

VESC_IF->io_set_mode(beeper_pin, VESC_PIN_MODE_OUTPUT);

And since beeper_update waits for beep_countdown to be 0 and does nothing to the pin after beep_alert() the pin remains high for that first beep_countdown period which makes that beep.

Also I wanted to try to build the package on MacOS which was successful but since vesc_tool path in MacOS contains spaces I had to wrap $(VESC_TOOL) in double quotes in Makefile. I double checked this change on Linux and it is not causing any issues.

[lukash]

Thanks! Looks good, just one small nitpick.

[lukash]

Is this always the path where VESC Tool is installed on MacOS? The command right above it is generic and applies to Mac as well, you're adding a special case of it. I'm not too fond of adding the special case, if someone is compiling the package he's probably savvy enough to know the path and the generic instruction should be enough.

If the path on MacOS is standardized and always the same, I'm not entirely against it but it should be labeled more specifically as a special case of the above command for MacOS, e.g.: "For macOS, the path to VESC Tool is as follows:"

I'd also slightly favor quoting the string here instead of ascaping the spaces:

make VESC_TOOL="/Applications/VESC Tool.app/Contents/MacOS/VESC Tool"

[0x41kravchenko]

Thank you for the feedback!
Yes, it is default path if VESC tool is installed with MacOS package from official VESC website. Of course if one would compile VESC Tool then the path can be different.
I like the idea with a special case label, also it will be an indicator for other contributors that building packages with MacOS VESC Tool is doable. I'll update README according to your example.
At first I didn't want to update the Makefile and tried quoting the path in cli but command substitution in Makefile still interprets path with spaces as multiple arguments. Here's a test with reverted change:

% git diff
diff --git a/Makefile b/Makefile
index 411013d..b069eff 100644
--- a/Makefile
+++ b/Makefile
@@ -9,9 +9,9 @@ all: refloat.vescpkg

 refloat.vescpkg: src lisp/package.lisp package_README-gen.md ui.qml
 ifeq ($(OLDVT), 1)
-       "$(VESC_TOOL)" --buildPkg "refloat.vescpkg:lisp/package.lisp:ui.qml:0:package_README-gen.md:Refloat"
+       $(VESC_TOOL) --buildPkg "refloat.vescpkg:lisp/package.lisp:ui.qml:0:package_README-gen.md:Refloat"
 else
-       "$(VESC_TOOL)" --buildPkgFromDesc pkgdesc.qml
+       $(VESC_TOOL) --buildPkgFromDesc pkgdesc.qml
 endif

 src:
 
% make VESC_TOOL="/Applications/VESC Tool.app/Contents/MacOS/VESC Tool"
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C src
arm-none-eabi-gcc alert_tracker.so atr.so balance_filter.so biquad.so bms.so booster.so brake_tilt.so charging.so data_recorder.so footpad_sensor.so haptic_feedback.so imu.so konami.so lcm.so led_driver.so led_strip.so leds.so main.so motor_control.so motor_data.so pid.so remote.so state.so time.so torque_tilt.so turn_tilt.so utils.so lib/circular_buffer.so conf/confparser.so conf/confxml.so conf/buffer.so ../vesc_pkg_lib//utils//rb.so ../vesc_pkg_lib//utils//utils.so -nostartfiles -static -mfloat-abi=hard -mfpu=fpv4-sp-d16 -mcpu=cortex-m4 -lm -Wl,--gc-sections,--undefined=init -T ../vesc_pkg_lib//link.ld -flto -o package_lib.elf
lto-wrapper: warning: using serial compilation of 2 LTRANS jobs
lto-wrapper: note: see the '-flto' option documentation for more information
/Applications/ArmGNUToolchain/14.2.rel1/arm-none-eabi/bin/../lib/gcc/arm-none-eabi/14.2.1/../../../../arm-none-eabi/bin/ld: warning: package_lib.elf has a LOAD segment with RWX permissions
arm-none-eabi-objdump -D package_lib.elf > package_lib.list
arm-none-eabi-objcopy -O binary package_lib.elf package_lib.bin --gap-fill 0x00
python3 ../vesc_pkg_lib//conv.py -f package_lib.bin -n package_lib > package_lib.lisp
/Applications/VESC Tool.app/Contents/MacOS/VESC Tool --buildPkgFromDesc pkgdesc.qml
make: /Applications/VESC: No such file or directory
make: *** [refloat.vescpkg] Error 1

[lukash]

Nono, I didn't mean to remove the quotes in the Makefile (those are needed), I meant to put this into the README:

make VESC_TOOL="/Applications/VESC Tool.app/Contents/MacOS/VESC Tool"

That is, quotes instead of the backslashes. This is more conventional and it's easier to change the path to another path with spaces, so it's better to have it that way for documentation.

[lukash]

Oh, for the beeper fix, please add a changelog record via a git trailer as described in https://github.com/lukash/refloat/blob/main/CONTRIBUTING.md

You can also find examples earlier commits.

[0x41kravchenko]

@lukash Updated the PR according to the nitpicks 👌