Skip to content

Add json write support - #13

Merged
AndreFavotto merged 4 commits into
epics-modules:mainfrom
essioc:merge-json-write-support
May 13, 2026
Merged

Add json write support#13
AndreFavotto merged 4 commits into
epics-modules:mainfrom
essioc:merge-json-write-support

Conversation

@essioc

@essioc essioc commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

This changelist adds JSON write, using existing value conversion operation used for FLAT write.

@essioc
essioc force-pushed the merge-json-write-support branch from 3543d8b to 67de182 Compare April 27, 2026 15:51
@essioc
essioc marked this pull request as ready for review May 5, 2026 13:07
@essioc
essioc marked this pull request as draft May 5, 2026 13:14
@essioc

essioc commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

@AndreFavotto Hi! I have successful ran this implementation using a MQTT based device. However I was not able to get a test setup working, so the Json write features are not covered. Would be nice to get this working also

@AndreFavotto

AndreFavotto commented May 5, 2026

Copy link
Copy Markdown
Collaborator

@essioc great, thanks for working on that. Can you rebase main into this branch now that #12 is merged?
I'll take a look on the test cases.

@essioc
essioc force-pushed the merge-json-write-support branch from 76725ce to facd3b0 Compare May 5, 2026 13:41
AndreFavotto
AndreFavotto requested changes May 13, 2026
View reviewed changes
Comment thread mqttSup/src/drvMqtt.cpp
AndreFavotto
AndreFavotto reviewed May 13, 2026
View reviewed changes
Comment thread mqttSup/src/drvMqtt.cpp
AndreFavotto
AndreFavotto reviewed May 13, 2026
View reviewed changes
Comment thread testApp/Db/mqttTest.db Outdated
AndreFavotto
AndreFavotto reviewed May 13, 2026
View reviewed changes
Comment thread tests/test_mqtt_roundtrip.py Outdated
AndreFavotto
AndreFavotto reviewed May 13, 2026
View reviewed changes
Comment thread mqttSup/src/drvMqtt.cpp Outdated
@AndreFavotto

AndreFavotto commented May 13, 2026

Copy link
Copy Markdown
Collaborator

I'll cleanup the commit history here for conciseness

essioc and others added 4 commits May 13, 2026 10:54
@essioc @AndreFavotto
feat: implement mqtt write
9af6238 
Co-authored-by: André Favoto <andre.favoto@ess.eu>
@essioc @AndreFavotto
test: add test for JSON records
ed08953 
Co-authored-by: André Favoto <andre.favoto@ess.eu>
@essioc @AndreFavotto
doc: update documentation about JSON write feature
45cd00b 
Co-authored-by: André Favoto <andre.favoto@ess.eu>
@AndreFavotto
docs: update instructions on how to run tests locally
5a24ea2
@AndreFavotto
AndreFavotto force-pushed the merge-json-write-support branch from 95337ed to 5a24ea2 Compare May 13, 2026 09:13
@AndreFavotto
AndreFavotto marked this pull request as ready for review May 13, 2026 09:15
AndreFavotto
AndreFavotto approved these changes May 13, 2026
View reviewed changes

@AndreFavotto AndreFavotto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@essioc LGTM now, I'll wait for your input before merging!

@essioc

essioc commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Looks like there was yet another asynSuccess flag I had to set that I missed!

I suggest checking autoformat settings, I see that some extra formatting is getting into the readme file unrelated to the merge. Nonetheless it looks good.

@AndreFavotto

AndreFavotto commented May 13, 2026

Copy link
Copy Markdown
Collaborator

I suggest checking autoformat settings, I see that some extra formatting is getting into the readme file unrelated to the merge.

Yeah, I ran autoformatter locally before pushing because I notice the file was not formatted before. There were some very big lines, as this one: https://github.com/epics-modules/mqtt/pull/13/changes#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5L122, whereas some places like the copyright notice it was cropped at 80 columns.
I reformatted all to 120 columns now, but I will work on making this more consistent throughout the repo and enforcing some pre-commit rules.

@AndreFavotto
AndreFavotto merged commit 56112dc into epics-modules:main May 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndreFavotto AndreFavotto AndreFavotto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@essioc @AndreFavotto