DeviationTX / deviation

Custom firmware for RC Transmitters
http://www.deviationtx.com
GNU General Public License v3.0
247 stars 153 forks source link

New method for handling custom alarms #961

Open eried opened 4 years ago

eried commented 4 years ago

This fixes https://github.com/DeviationTX/deviation/issues/960

Available settings in src/music.c: USE_NEW_CUSTOM_ALARM_MODE, if is 0 then the behaviour is exactly the same as the original

CUSTOM_ALARM_IGNORE_MS, is the time in ms that the sound will not be triggered. So if is 200 ms and the user switch from A to C, sound B wont be played if the user took less than 200 ms on moving the switch from B to C.

eried commented 4 years ago

I like the general idea, but I'd suggest working this into the config (tx.ini?) files instead of the define and also add menu options for setting this up? Maybe we can have the "old" behavior when the value for CUSTOM_ALARM_IGNORE_MS is "0"?

I agree that a menu entry would be ideal, I am going to check that section on the source. I left that as a define thinking that in the unlikely case that someone might want that old behaviour, it still could be engaged.

Edit: Another thing comes to my mind: Maybe ignoring all custom alarms coming in fast isn't desirable: Imaging flipping a switch immediately before a telemetry alarm with custom sound comes in. So maybe this behavior should be exclusive for switch custom alarms? I'm open for discussion on this matter.

You are right, I was not aware or explored those telemetry alarms and others. In this case it would require a "source" flag (i.e. priority and have that as a setting in the menu maybe?).

I am thinking a solution could be: 1) Add priority to the int AUDIO_AddQueue(u16 music) { function 2) Expose that priority in options: Priority for switch voice: high|normal

In that way the "buzzer" functionality (that was what I wanted to fix later) could use those rules.

I was just blinded for how cool it feels that the sounds are skipabble. https://youtu.be/Bhd3Ko32Q8U?t=264

TheRealMoeder commented 4 years ago

I agree that a menu entry would be ideal, I am going to check that section on the source. I left that as a define thinking that in the unlikely case that someone might want that old behaviour, it still could be engaged.

Menu is pretty simple; I can add that later in case you don't want to dig into the source too much.

You are right, I was not aware or explored those telemetry alarms and others. In this case it would require a "source" flag (i.e. priority and have that as a setting in the menu maybe?). I am thinking a solution could be:

  1. Add priority to the int AUDIO_AddQueue(u16 music) { function
  2. Expose that priority in options: Priority for switch voice: high|normal

I think this really only applies to switches, so we might as well make a seperate case for switches. How about somehow passing a switch id (or group) together with the voiceid for the queue voice to always replace queue entries for the same switch id/group so we only have the latest switch state in the queue? Will probably require a seperate AUDIO_AddQueueSwitch (u16 music u16 group) or similar. The reason behind this is, you could also be flipping to switches at the same time (not that uncommon in the heat of the flight) and wouldn't want to lose any of the announcements of either switch.

While at reworking, I think it also makes sense for AUDIO_AddQueue to check, whether the music being added to the queue is the same as the last entry in the queue, and in that case discard the duplicate entry.

In that way the "buzzer" functionality (that was what I wanted to fix later) could use those rules.

I was just blinded for how cool it feels that the sounds are skipabble. https://youtu.be/Bhd3Ko32Q8U?t=264

I wasn't aware that someone finally found a fix for the "saving" issue - great!

@goebish you always have some good ideas, any comments on this issue?

eried commented 4 years ago

Menu is pretty simple; I can add that later in case you don't want to dig into the source too much.

That would be awesome.

While at reworking, I think it also makes sense for AUDIO_AddQueue to check, whether the music being added to the queue is the same as the last entry in the queue, and in that case discard the duplicate entry.

Not only duplicates, but also how many ms since it was triggered, to skip the sounds while switching a 3 state switch. I do not really agree with keeping ALL the switches voices all the time, I like how it drops or interrupts voices. But ofc this would need some kind of survey, or option.

I wasn't aware that someone finally found a fix for the "saving" issue - great!

Yes, sadly is a hardware issue that will need a pcb revision.

TheRealMoeder commented 4 years ago

While at reworking, I think it also makes sense for AUDIO_AddQueue to check, whether the music being added to the queue is the same as the last entry in the queue, and in that case discard the duplicate entry.

Not only duplicates, but also how many ms since it was triggered, to skip the sounds while switching a 3 state switch. I do not really agree with keeping ALL the switches voices all the time, I like how it drops or interrupts voices. But ofc this would need some kind of survey, or option.

Well, the switch issue would be solved with adding the alarm group to a queue entry. Which means an alarm from the same group will cancel old alarms in the queue with the same group id. At least that's how thought of it. Do you get what I mean?

eried commented 4 years ago

Yes I understand about the "groups", that concept makes skipping voices harder to implement, and it just solves the "switching from A to C without hearing B".

Just for the sake of it, I am going to list the cases: 1) Able to skip currently playing voices: video

2) Except if is an alarm or some important stuff: video

Those two are common sense, and not open for discussion, right? 3) Skip voices from the same switch: video

This is also common sense. However: 4) Skip pending voices, even if they are not from the same switch: video

In my opinion, in that example the user should not have to hear all the switches positions in a queue. That could be the setting of "priority of switch position", however I still do not see much of a point of enqueueing (and hearing) all voice switches.

TheRealMoeder commented 4 years ago
  1. Able to skip currently playing voices:
  2. Except if is an alarm or some important stuff: Those two are common sense, and not open for discussion, right?

Right, would be solvable with the switch group concept.

3) Skip voices from the same switch:

This was what started the whole discussion, so nothing to add here.

4) Skip pending voices, even if they are not from the same switch: In my opinion, in that example the user should not have to hear all the switches positions in a queue. That could be the setting of "priority of switch position", however I still do not see much of a point of enqueueing (and hearing) all voice switches.

This is the only case which would need the priority setting. But I'm having some issues about making setting each alarms priority a requirement. From a user perspective, I think telemetry, timer and battery alarms are all somehow high priority unless specifically changing this behavior, so this will probably be the default behavior. Your wish number 4 could be achievable by having the option to "upgrade" specific switch settings to the same high priority level as the ones above.

I do not think we can omit enqueuing all switches, as this will make the behavior somewhat unpredictable and I again think of the case of dual switch flips at the same time.

eried commented 4 years ago

I am still thinking on a solution, so question for you @TheRealMoeder, Imagine 2 switches, you switch one, voice start playing... switch the 2nd one. Does the first voice gets interrupted?

TheRealMoeder commented 4 years ago

I am still thinking on a solution, so question for you @TheRealMoeder, Imagine 2 switches, you switch one, voice start playing... switch the 2nd one. Does the first voice gets interrupted?

I can imagine both behaviors, but I tend to prefer the first voice not being interrupted