Skip to content

Conversation

@PieterGit
Copy link
Contributor

@PieterGit PieterGit commented Dec 25, 2018

With a fresh install with a WW pump, the rig could not communicate to the pump.

  • removing python oref0_init_pump_comms.py and oref0_subg_ww_radio_parameters.py (not needed with Go libaries)
  • use .travis with latest Node 8 LTS and Node 10 LTS versions
  • docfix: update version numbers of Go libraries to version with tokenbased authentication
  • remove --ww_ti_usb_reset
  • remove oref0 dependencies on pump.ini
  • fix jq install problem, and don't install master version of oref0 by default on bin/openaps-packages.sh. It will be installed by oref0-setup.sh
  • read pump.ini stuff from preferences.json instead of pump.ini
  • mmtune fix if does not exist on install
  • mmtune fix: die in bin/oref0-mmtune.sh if monitor/mmtune.json is empty or does not exist
  • fix for mmeowlink if you're not a mdt cgm user
  • removed radio_locale pump.ini hacks
  • update npm libraries

Status: Ready for merge to dev, when @scottleibrand thinks it's ready.

PieterGit and others added 7 commits December 25, 2018 18:22
…s a switch in preferences.json.

WIP to remove use of pump.ini in favor of storing pump serial, radio locale, and tty port in preferences.json.
this removes warning: npm WARN deprecated [email protected]: This package is no longer supported. It's now a built-in Node module. If you've depended on crypto, you should switch to the one that's built-in.
jq: Unknown option --slurpfile
Use jq --help for help with command-line options,
or see the jq documentation at http://stedolan.github.com/jq
module.js:675
    throw err;
    ^

SyntaxError: /root/myopenaps/updated_prefs.json: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at Object.Module._extensions..json (module.js:672:27)
    at Module.load (module.js:566:32)
    at tryModuleLoad (module.js:506:12)
    at Function.Module._load (module.js:498:3)
    at Module.require (module.js:597:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/.rootfs/usr/local/lib/node_modules/oref0/bin/oref0-get-profile.js:81:23)
    at Module._compile (module.js:653:30)
    at Object.Module._extensions..js (module.js:664:10)
Could not run oref0-get-profile

---

# jq --version
jq-1.4-1-e73951f
Copy link
Contributor

@scottleibrand scottleibrand left a comment

Choose a reason for hiding this comment

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

I don’t think changing the install script names is necessary or necessarily a good idea. Coordinating changes between the code and documentation, especially when changes made to dev don’t go to master right away, is difficult and causes confusion.

@PieterGit
Copy link
Contributor Author

@scottleibrand I think i fixed all your open review remarks. Can you please re-rereview. I will test on a fresh edison and rpi later.

…the major changes and could not find deprecations we use
@scottleibrand
Copy link
Contributor

scottleibrand commented Feb 17, 2019

Looks like there's a conflict between @cclauss' and @PieterGit's recent changes to .travis.yml.
@PieterGit did:

node_js:
  - 8
  - 10

whereas @cclauss did:

matrix:
  include:
    - node_js: "8.11.1"
      <<: *node_js-steps

Which do y'all prefer?

scottleibrand
scottleibrand previously approved these changes Feb 17, 2019
Copy link
Contributor

@scottleibrand scottleibrand left a comment

Choose a reason for hiding this comment

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

Thanks @PieterGit - once the merge conflict is fixed, this LGTM.

@cclauss
Copy link
Contributor

cclauss commented Feb 17, 2019

#1203 Perhaps:

matrix:
  include:
    - node_js: “8”
      <<: *node_js-steps
    - node_js: “10”
      <<: *node_js-steps
    - node_js: “node”
      <<: *node_js-steps

```
# oref0-mmtune
mmtune: WW
2019/02/17 23:04:04 cannot connect to CC111x radio on /dev/spidev5.1
2019/02/17 23:04:04 cc111x: no response
Usage: grep [OPTION]... PATTERN [FILE]...
Try 'grep --help' for more information.
```
@scottleibrand scottleibrand dismissed their stale review February 20, 2019 05:02

@PieterGit isn't ready to merge yet

…eeded for mdt CGM, try to see if we can get mmeowlink back to work if users specifies port with mmeowlink in it 3) add todo's
@PieterGit
Copy link
Contributor Author

PieterGit commented Feb 20, 2019

spi_serial and mraa don't seem to be needed for 754WW pump

@scottleibrand Tested on a explorer board and with WW-pump and it works. I think it's r eady for further test, review and merge.
@cluckj can you also review this PR?
@tepidjuice can you see if you're radio comms problems is solved with this PR?

grep -q radio_locale pump.ini || echo "$(< pump.ini)" > pump.ini ; echo "radio_locale=$radio_locale" >> pump.ini
fi
else
else # in all other case create a pump.ini with serial and radio_locale. TODO: can we skip creating a pump.ini. I can't find references of it's being used
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that xdrip-js uses pump.ini, we should let them know before removing it: @efidoman @jpcunningh @thebookins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't want to suggest to remove it now. As far as I can see I removed all the references to the pump.ini from the oref0 code base. I hope @efidoman @jpcunningh @thebookins can say if their software somehow uses pump.ini. I just checked Lookout, but can't find any reference: https://github.com/xdrip-js/Lookout/search?q=pump.ini&unscoped_q=pump.ini

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created xdrip-js/Logger#137 for that

@PieterGit
Copy link
Contributor Author

PieterGit commented Feb 24, 2019

rig is running fine for 4 days. Didn't find issues. I think this PR is ready to merge to dev.
@scottleibrand can you review and merge to dev when you think it's ready?

@PieterGit
Copy link
Contributor Author

I resolved the conflict with the dev branch. Rig is used as primary rig, and didn't find issues for a week.
@scottleibrand Can you please review and merge to dev if you think it's ok?

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Comma missing at end of line 110?

@PieterGit
Copy link
Contributor Author

@cclauss thanks. fixed it with the Github UI. re-applied.
@scottleibrand please merge to dev

@scottleibrand
Copy link
Contributor

This LGTM. Once we iron out the issues with dev and decide if webpack is staying or going, I'll fix the merge conflicts and confirm tests pass before merging.

@scottleibrand scottleibrand merged commit 850545b into openaps:dev Mar 10, 2019
@PieterGit
Copy link
Contributor Author

Thanks for merging. Up to the next PR.

@efidoman
Copy link
Contributor

efidoman commented Apr 1, 2019

I remembered that having this variable set is no longer necessary for fakemeter, so I tested Logger with that line deleted and fakemeter works fine. In addition, the fakemeter processing has moved in the latest Logger and OpenAPS dev branches to process as part of the OpenAPS loop. I'll update the Logger code and remove this line.

@efidoman
Copy link
Contributor

efidoman commented Apr 1, 2019

All references to pump.ini have been removed from the Logger dev branch.

scottleibrand pushed a commit that referenced this pull request Apr 16, 2019
* Fix for x12 status check

* Spacing

* Remove workarounds for missing x12 settings

* Remove workarounds for missing x12 settings

* Spacing...

* Improving automation of hardware setups, support for more kinds of Go binaries, x12 cleanup.

* Update installer options, deprecate MDT, conflict changes with #1176

* Deprecate G4-upload and G4-local-only, setup readability

* Bugfixes, cleanup, and deprecate mmeowlink...

* Refactor interactive setup...

* Refactor, can ensure building from source for certain setups without binaries

* Better support for custom radiotags and diy radios

* Comments, and activate SPI only on the Pi

* Typo...

* Restore mmeowlink

* Missing quotation marks

* Move updating jq earlier in the install script

Some jq calls between lines 683 and 1132 need `sub`, which is only supported in newer versions of jq...

* Revert to original repo, and do the config change with sed

* Additional comments, remove redundant jq installs
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.

5 participants