mysensors / MySensors

MySensors library and examples
https://www.mysensors.org
1.3k stars 891 forks source link

Multiple Improvements and bugfix on WaterMeterPulseSensor example #1540

Closed paolo-rendano closed 1 year ago

paolo-rendano commented 1 year ago
paolo-rendano commented 1 year ago

This has been tested on a real working water meter pulse sensor with

on an Nano board with a NRF24L01+ module connected to HA via serial gateway (same hardware of the sensor)

mfalkvidd commented 1 year ago

Nice work!

One thought: Most Arduino libraries have "simple / bare bones" examples and advanced examples. The simple ones are easier to understand (especially for people just starting to use the library), while the advances ones are great if you want to use the full feature set.

I'm not sure the previous version of this example was simple enough to qualify for the "simple" category, but with the added 90 lines this example is now almost 60% larger which (imo) clearly puts it into the "advanced" category.

Should we split this example into two: one simple and one advanced?

paolo-rendano commented 1 year ago

Hello Mikael, thanks. About your question, we could also split that but, anyway in this enhancement, apart adding new features like the correction about the counter or the initial provisioning, there are important fixes that make the sketch usable in very simple steps (the current one does not work out of the box but needs several headaches to understand many things before being able to use that. I summarized all these headaches in this new sketch to make things simpler for one who wants to use that sketch. Now, given that, if the existing one (the "basic") is designed to make people incepting the mysensors project, debug, study and understand many things, yes, this could give the option for the skilled consumer to reach this objective. Whilst, if the consumer is not skilled to change it but needs to download and use it out of the box, then the best is the new one (Note: i have also some descriptions and screenshot to attach somewhere, maybe in the description page, not yet done, i need to understand where to commit these changes).

mfalkvidd commented 1 year ago

Do you mean https://www.mysensors.org/build/pulse_water ?

It is possible to add information to that page. The page is stored in the MySensors content management system. I don't know how access permissions are handled but it is probably possible to find a way to let you contribute to it.

paolo-rendano commented 1 year ago

Do you mean https://www.mysensors.org/build/pulse_water ?

It is possible to add information to that page. The page is stored in the MySensors content management system. I don't know how access permissions are handled but it is probably possible to find a way to let you contribute to it.

yes exactly

mfalkvidd commented 1 year ago

@henrikekblad do we have a good way to give @paolo-rendano edit access to https://www.mysensors.org/build/pulse_water ?

paolo-rendano commented 1 year ago

@henrikekblad do we have a good way to give @paolo-rendano edit access to https://www.mysensors.org/build/pulse_water ?

@henrikekblad may I have access to CMS for updating the page, so that also the sketch can be merged?

henrikekblad commented 1 year ago

Hi @paolo-rendano. The content management system is a bit clunky regarding rights management.

Maybe you could fork a gist I've created with the page markup (keep the images even if they are broken) and make the changes. I will paste them back into the content management system when you're done. https://gist.github.com/henrikekblad/aecfc4fc82dac39a39376e89a70115c1

Cheers Henrik

paolo-rendano commented 1 year ago

Hi @paolo-rendano. The content management system is a bit clunky regarding rights management.

Maybe you could fork a gist I've created with the page markup (keep the images even if they are broken) and make the changes. I will paste them back into the content management system when you're done. https://gist.github.com/henrikekblad/aecfc4fc82dac39a39376e89a70115c1

Cheers Henrik

Hello @henrikekblad. Could you please review my fork and give me your feedback? Of course the change to the page should be done in conjunction with the merge of the pull request.

Thanks Paolo

henrikekblad commented 1 year ago

I think you have to post a link to the gist-fork if it isn't public.

paolo-rendano commented 1 year ago

I think you have to post a link to the gist-fork if it isn't public.

Oh sorry, this is the link : https://gist.github.com/paolo-rendano/afc8dddd889e9463c3a1ea786c6b4352

henrikekblad commented 1 year ago

@paolo-rendano, CMS page updated.

Thanks for the contribution.

mfalkvidd commented 1 year ago

@hek should we change the cms page to point to the development branch for this sketch? Or should we create a mysensors release?

paolo-rendano commented 1 year ago

@hek should we change the cms page to point to the development branch for this sketch? Or should we create a mysensors release?

I guess this should be a release. currently what i can see in the cms page https://www.mysensors.org/build/pulse_water is that the page does not correspond with the example code

mfalkvidd commented 1 year ago

The downside with a release is that it is 100x more complicated than changing which branch to use for this specific sketch.

paolo-rendano commented 1 year ago

@paolo-rendano, CMS page updated.

Thanks for the contribution.

@hek could you please update the CMS page again. I've just realized to have missed the code block format for the yaml code. I've just fixed it in the gist. sorry, thanks