Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

playSound and setLedColor very unreliable #44

Open
theohm opened this issue Dec 14, 2020 · 11 comments
Open

playSound and setLedColor very unreliable #44

theohm opened this issue Dec 14, 2020 · 11 comments

Comments

@theohm
Copy link

theohm commented Dec 14, 2020

Libraries: Legoino 1.0.2, NimBLE-Arduino 1.0.2
ESP32 board: AZ-Delivery ESP32 NodeMCU V2
Lego model: Duplo Loco from the 10874 set

Cornelius, big thanks for your Legoino library!

When I use Legoino, the methods setBasicMotorSpeed and stopBasicMotor both work perfectly every time. On the other hand, playSound and setLedColor are very unreliable and only do their job in about one out of three times called. Any ideas what I do wrong? Is there a known bug? Maybe in conjunction with certain ESP32 boards?

Attaching an .ino file renamed to .txt: Simplified.ino

@corneliusmunz
Copy link
Owner

corneliusmunz commented Dec 14, 2020

Hi @theohm ! Thanks for your feedback and thanks for opening your first issue 👏 ! I highly appreciate any feedback on the library 👍

I also spotted some unreliable behaviour in playing sounds for the duplo train hub. I will have a more deeper look on that issue. I will try it out with another library (.net and node) and see if the behaviour is better. Your *.ino sketch looks quite ok.

@corneliusmunz
Copy link
Owner

@theohm Have you tried the example *.ino sketch which i have send to you? Was it better with removing the delays out of your sketch?

@theohm
Copy link
Author

theohm commented Dec 23, 2020

@corneliusmunz Sorry, I missed your message with the new sketch. How did you send it?

Removing the delay from my *.ino sketch would also disable debouncing of the buttons, so I did not completely remove it, but reduce it from 200 to 10. That did not change the behaviour.

Then I saw that there is a new Legoino version avaiblable in the Arduino IDE. So i updated to Legoino 1.0.2, which improved the reliability a bit. Now, only approximately every 3rd sound command is lost. After returning to a delay of 200, not even every 2nd sound command is executed, so the delay seems to affect the reliability a little bit. Reliability at a delay of 500 does not significantly differ from that at a delay of 200.

@corneliusmunz
Copy link
Owner

@theohm I have send you a mail with the sketch... maybe it is in the spam folder

@corneliusmunz
Copy link
Owner

Hi @theohm
i have had a look on your sketch and added the following sketch as example with the JC_Button library (just rename it to *.ino):
DuploHornButtonTest.txt
With this sketch almost every event will trigger a sound. Sometimes it does not work but i think this only happens if a sound already is playing on the hub and the hub will cancel duplicate events on the sound module and does not allow to trigger a sound while another sound output is still running.

@manuelm
Copy link

manuelm commented Dec 3, 2021

Hi there.

I recently build a remote control for my kid and noticed the same. Sound and color commands are unreliable and only work at about 2 out of 3 commands. However motor commands work all the time. So I looked at the bluetooth command bytes and noticed that motor commands are one bluetooth command (a single 0x81 port output command) vs sound/color are two (an additional 0x41 port input format setup beforehand). I then split the two commands into two functions and only sent the port input format after a successful connection to the hub. And now sound + color works 100% reliable.

I'm not sure why the current code always sends both commands. Imho it's not necessary.

So I basically did this:

class MyHub
  : public Lpf2Hub
{
public:
  void activateBaseSpeaker()
  {
    byte setSoundMode[8] = { 0x41, 0x01, 0x01, 0x01, 0x00, 0x00, 0x00, 0x01 };
    WriteValue(setSoundMode, 8);
  }

  void playSound(byte sound)
  {
    byte playSound[6] = { 0x81, 0x01, 0x11, 0x51, 0x01, sound };
    WriteValue(playSound, 6);
  }

  void activateRgbLight()
  {
    byte port = getPortForDeviceType((byte)DeviceType::HUB_LED);
    byte setColorMode[8] = { 0x41, port, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00 };
    WriteValue(setColorMode, 8);
  }

  void setLedColor(Color color)
  {
    byte port = getPortForDeviceType((byte)DeviceType::HUB_LED);
    byte setColor[6] = { 0x81, port, 0x11, 0x51, 0x00, color };
    Lpf2Hub::WriteValue(setColor, 6);
  }
}
void loop()
{
  if (myHub.isConnecting())
  {
    myHub.connectHub();
    if (myHub.isConnected())
    {
      [...]
      myHub.activateRgbLight();
      delay(200);
      myHub.activateBaseSpeaker();
      delay(200);
      [...]
    }
  }

  if (myHub.isConnected())
  {
    [...]
  }

@corneliusmunz
Copy link
Owner

@manuelm Many thanks for that finding! 👏 👍 Very good question why i have implemented this in that way. I think i have used the implementation of https://github.com/nathankellenicki/node-poweredup/blob/master/src/devices/duplotrainbasespeaker.ts as a basis but here the state of the activated mode is checked and if the mode is equal to the current mode, no message is sent out. So maybe i will change your implementation a littlebit and try to store the state. Then i will only send the message if the state differs to the current state.

@manuelm
Copy link

manuelm commented Dec 3, 2021

Sure. Just make sure to send both commands with a short delay. Wasn't reliable otherwise.

@theohm
Copy link
Author

theohm commented Dec 6, 2021

Thanks @manuelm for your solution!
@corneliusmunz: Will you eventually patch Manuels code into the official release?

@corneliusmunz
Copy link
Owner

@theohm I have already a local fix which is very similar to the solution from @manuelm I have to do some additional testing but I will definitely release it hopefully in the next days

@yschroeder
Copy link

I added the above implementation to my sketch and can confirm it is working well!

Would be great if this would be released as a bugfix release.

Thank you for the great library! I am currently building a remote control for my son's train, so right now I am having fun building it and then he will be having fun using it 😃

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

No branches or pull requests

4 participants