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

Update constant detections in MQTT (v2) #919

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

avigeilpro
Copy link
Contributor

Solved issue with long arguments.
Made possible to put curly brackets around any constant.
self tests successfull (had issue to configure visual studio because of missing nfd_d.lib but found how to compile)
tested on BK7231N and BK7231T with success.
here some examples of possibilities :
publish OpenBeken/test ${IP}_${CH4} 1
publish OpenBeken/test/${IP} ${CH4} 1
publish OpenBeken/test/$IP $CH4 1
publish OpenBeken/test ${IP}_ON 1

if the constant is the last before a space, you can write it like this $IP or ${IP} (both works)
if you have text after the constant, you must put the curly brackets like this ${IP}_ON

token with constants are limited to 40 characters (after expand). we could make it longer but we will have to extend the size of array g_argsExpanded[MAX_ARGS][40]. I assume it needs to stay so because of memory limitation.

If you see a case i didn't cover, please say me.

@avigeilpro avigeilpro changed the title Mqtt constants Update constant detections in MQTT (v2) Sep 11, 2023
@openshwprojects
Copy link
Owner

I am wondering whether we could merge it with CMD_ExpandConstantsWithinString

@avigeilpro
Copy link
Contributor Author

avigeilpro commented Sep 13, 2023

I took a look at CMD_ExpandConstantsWithinString, one problem that i see is that my code is more "universal", it means that on the self test "ba$CH11ha" for example, it will try find the constant named $CH11ha, it dont know that a channel can have only 2 digits, all it do is cutting with a patern. To have the same result with my code you should put "ba${CH11}ha". I made this because if somebody want send the value of CH1 followed by a "0" for example, with CMD_ExpandConstantsWithinString it's not possible (or i missed something). If this is a problem, i can try do something, but i think this solution is better for futur development, and it is not something new as it is the way we do in JS to include variables in strings.

@openshwprojects
Copy link
Owner

So basically, we'll migrate to your solution? Or keep both of them for now?

@avigeilpro
Copy link
Contributor Author

This depend of your will. I don't know if the formatting like "baCH11ha" is already used or not. If not, i think you should migrate to "my" solution, but if it is used and is difficult to modifie, you should keep both, and slowly migrate all the function wich use the original formatting. Say me what you prefer, and i will rewrite the code in consequence.

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

Successfully merging this pull request may close these issues.

None yet

2 participants