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

Added support for custom rfswitch protocol definition (id = 0) #2255

Conversation

thornley-david
Copy link

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.

Exposes the ability to configure a custom protocol definition
Add support for start/stop bit framing
Fixed issue with gpio line kept high (keying/flooding radio)

Added setcustom() function which is assigned to protocol 0, these values can be tweaked in lua rather than derived from a predefined table.
Added optional frameStartStop to cater for Arlec style R213 waveforms which are framed 32 bits around additional start and stop bits (34 signals in total). Can be 0=NONE, 1=START, 2=STOP, 3=START&STOP
Added ability to invert startStop bits (logic signal one rather than zero).
Fixed issue with gpio pin kept high after repeat waveforms are sent and invertSignal is true (floods rf band). The gpio pin is now always assigned to logic 0 once send() is completed.
@thornley-david thornley-david changed the base branch from master to dev February 6, 2018 13:33
transmit(p.syncFactor, p.invertedSignal, pulse_length, pin);
}
platform_gpio_write(pin, 0); // If we leave gpio 0 high on last transmit, we flood the rf band (so we should always set low this after sending waveforms)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking back at the previous fix #1738, the correct thing to do here might be to set the GPIO pin state to p.invertedSignal

platform_gpio_write(pin, p.invertedSignal);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this again, and in one of my usage scenarios I actually set invertedSignal to true. Since my hardware's 433 transmitter is active high, it should still 0 as I have set it here. @ffedoroff who authored #1738 might be able to comment on whether or not this is still sufficient for his scenario given he originally moved away from this approach.

app/modules/rfswitch.c Outdated Show resolved Hide resolved
@nodemcu nodemcu deleted a comment from tongtong15 Feb 8, 2018
@nodemcu nodemcu deleted a comment from tongtong15 Feb 8, 2018
@thornley-david
Copy link
Author

fixed that embarrassing assignment mishap ;)

};

Protocol custom = { 350, { 1, 31 }, { 1, 3 }, { 3, 1 }, false, false }; // Defaults to same as protocol 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if this was the same as protocol 1 -- there is a missing 0 near the end of the initialization. It doesn't really make a difference, but it ought to be fixed.

@@ -76,19 +98,25 @@ void transmit(HighLow pulses, bool invertedSignal, int pulseLength, int pin) {
*/
void send(unsigned long protocol_id, unsigned long pulse_length, unsigned long repeat, unsigned long pin, unsigned long value, unsigned int length) {
platform_gpio_mode(pin, PLATFORM_GPIO_OUTPUT, PLATFORM_GPIO_FLOAT);
Protocol p = proto[protocol_id-1];
Protocol p = custom;
if(protocol_id > 0) p = proto[protocol_id-1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a check for having the protocol id in range somewhere? It should probably throw a lua error if it is out of range. Otherwise I suspect that rfswitch.send(9999999, ...) will cause the system to fault.

@marcelstoer
Copy link
Member

@landswipe do you plan to address the latest comments by @pjsg?

@thornley-david
Copy link
Author

Yes I will... I had forgotten about this, thanks for the reminder :)

@marcelstoer
Copy link
Member

This is now stale and has merge conflicts. Shall we close it?

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@stale stale bot closed this Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants