Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Define to disable ON/OFF beeps / optimisations #66

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

EmerickH
Copy link
Member

@EmerickH EmerickH commented Jul 21, 2019

In this PR:

  • Added BEEPS_ON_OFF constant to enable/disable beeping at start/stop
  • Added ENABLE_INACTIVITY_TIMEOUT, this does the same thing as disablepoweroff variable but for people that really don't need it it can speed up main loop
  • Optimized disable functions:
#if BEEPS_BACKWARD // Does not compile this part if disable
    } else if (speed < -50) {  // backward beep
      buzzerFreq = 5;
      buzzerPattern = 1;
#endif

instead of

    } else if (BEEPS_BACKWARD && speed < -50) {  // Compiled and calculated at every loop even if disabled
      buzzerFreq = 5;
      buzzerPattern = 1;

for example

  • Fixed WHEEL_SIZE_INCHES to be defined like others config defines in config
  • SOFTWARE_SERIAL_BAUD was declared 2 times in config file
  • Fixed battery raw reading not updating
  • Removed the old unnecessary INCLUDE_PROTOCOL2 (see also Remove INCLUDE_PROTOCOL2 bipropellant-protocol#30)

@p-h-a-i-l
Copy link
Member

Is there a need to speed up the main loop?

I'd rather eliminate all the preprocessor switches than add new ones.
Let's even go further and use Variables for WHEEL_SIZE_INCHES, BEEPS_BACKWARD, BEEPS_ON_OFF, TEMP_POWEROFF_ENABLE and TEMP_WARNING_ENABLE, store their values in flash and use the protocol to change them.

@p-h-a-i-l
Copy link
Member

Whoops, just wanted to make a PR to your branch but commited directly by accident (why do I even have the right to do that? ;) )

Anyways, changed your platformio.ini to be compatible to the new config structure.

@EmerickH
Copy link
Member Author

EmerickH commented Aug 6, 2019

Yes also did this exact change but not pushed it yet.
For the preprocessor switches, the goal is not really to speed up the main loop but just to disable functions that I will never need and so there's no interest for me to compile them.
Maybe for battery/temp calibration we could move them flash instead, because it's specific to each board, but for enabling beeps, I don't really see the interest. It's even more complicated to send the value & the flash magic by serial that just disabling it in your config

@p-h-a-i-l
Copy link
Member

I guess @btsimonh and I developed a heavy dislike against using preprocessor switches in the last months ;)

Lots of (maybe) dead code and too many possible configurations which can interfere. Impossible to test.

By putting everything in variables and having a runtime config you can still set the options while flashing. As soon as the "magic" changes, defaults are used.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants