pdxlocations / MQTT-Connect-for-Meshtastic

GNU General Public License v3.0
99 stars 14 forks source link

field entry validation #17

Open pdxlocations opened 10 months ago

william-stearns commented 3 months ago

Was this a placeholder for something specific, or a request to check all entered fields for valid format from the UI?

pdxlocations commented 3 months ago

Both, there are currently no checks to validate input for the expected length/values.

william-stearns commented 3 months ago

It doesn't appear at first glance that there's anything in the standard library to do this kind of validation so whatever approach we consider will likely need the end user to install at least one python library (imho, since the user will already have to had installed meshtastic, this shouldn't be a big problem.) pydantic keeps coming up for a number of reasons (popularity, speed of execution, and using the type hints that are a good idea in the first place in python code.) What would you think of me taking this approach: adding type hinting to functions and variables, then adding pydantic field checking?
(For a straightforward example on how it's used, see https://docs.pydantic.dev/latest/concepts/validation_decorator/#function-signatures .)

pdxlocations commented 3 months ago

I was thinking of just very basic validation that wouldn't need additional libraries: Is the Node ID valid Hex and the correct length? Is the outgoing message too long?

However, I'm not opposed to anyone taking an interest in improving the script in any way that interests them! I'll merge any PR that improves the project.

william-stearns commented 3 months ago

Sounds like a plan - I can start with those two requests. Are you OK with me later adding type information (like longname: str )? If so, I assume you'd like that as a separate patch so you can review it separately?

pdxlocations commented 3 months ago

I'm not familiar with applying patches. Could you submit PRs?

william-stearns commented 3 months ago

Glad to. Would you prefer I do my work on a fork or send the PR directly to your repo? (Please note: I am not asking for and do not want the ability to actually make changes to your repo. I prefer to have a second set of eyes on anything I submit.)

william-stearns commented 3 months ago

First pass is at https://github.com/pdxlocations/Meshtastic-MQTT-Connect/pull/54 . (Sorry, I hadn't realized I could create a PR across forks!)